Skip to content

Preferences: preserve unknown WebServer Template across GUI Save (#267) - #621

Merged
mrjimenez merged 1 commit into
amule-project:masterfrom
got3nks:fix/prefs-skin-template-preserve
May 15, 2026
Merged

Preferences: preserve unknown WebServer Template across GUI Save (#267)#621
mrjimenez merged 1 commit into
amule-project:masterfrom
got3nks:fix/prefs-skin-template-preserve

Conversation

@got3nks

@got3nks got3nks commented May 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Editing any preference in amulegui and clicking Save silently clears Template=AmuleWebUI-Reloaded (or any other template name) from amule.conf, falling back the web UI to the default skin until the user re-edits the config by hand. ngosang isolated the symptom in #267.

Root cause

Cfg_Skin::TransferToWindow populates the prefs-dialog wxChoice by scanning the local webserver / skin directories on the machine running amulegui. When the configured value isn't found in the local listing:

int id = skinSelector->FindString(m_value);
if ( id == wxNOT_FOUND ) {
    id = 0;
    if (m_is_skin) {
        m_value = defaultSelection;
    }
}
skinSelector->SetSelection(id);

FindString returns wxNOT_FOUND, the dropdown silently selects index 0 (typically empty or "no options available"), and the subsequent TransferFromWindow at Save time reads that back into m_value. The "value resets to whatever the widget shows" pattern erases the on-disk Template.

This bites users whose:

  • amulegui is on a different host from amuleweb (the GUI's local dirs don't list templates installed only on the daemon host)
  • templates live outside the dirs the GUI scans (e.g. via Docker volume mounts that target /usr/local/share/... while the GUI scans the user's ~/.aMule/webserver/)

Fix

The two Cfg_Skin sites have different semantics:

  • /SkinGUIOptions/Skin renders inside amulegui itself — a local fallback to "- default -" is correct, the GUI genuinely can't load a skin file it doesn't have.
  • /WebServer/Template is consumed entirely by amuleweb, which may be on a different filesystem from amulegui. The GUI has no business overwriting a value it doesn't recognise.

For the template case, append the unrecognised non-empty value to the dropdown and select it, so TransferFromWindow reads back the same string and Save becomes a no-op:

int id = skinSelector->FindString(m_value);
if ( id == wxNOT_FOUND ) {
    if (m_is_skin) {
        id = 0;
        m_value = defaultSelection;
    } else if (!m_value.IsEmpty()) {
        id = skinSelector->Append(m_value);
    } else {
        id = 0;
    }
}
skinSelector->SetSelection(id);

Skin behaviour is unchanged. Template values that genuinely are present locally still match via FindString and follow the existing path.

Validation

Closes #267.

@got3nks
Preferences: preserve unknown WebServer Template across GUI Save (amu…
514dc9f 
…le-project#267)

Cfg_Skin::TransferToWindow populates the prefs-dialog wxChoice by
scanning the *local* webserver and skin directories. When the
configured value isn't found in the local listing -- e.g. when
amulegui runs on a different host than amuleweb, or templates live
outside the dirs the GUI scans -- FindString returns wxNOT_FOUND and
the dropdown silently defaults to index 0. The subsequent
TransferFromWindow at Save time reads index 0 back into m_value,
overwriting amule.conf's Template=AmuleWebUI-Reloaded with an empty
string. The webserver then falls back to the default template until
the user re-edits the config file by hand.

The two Cfg_Skin sites have different semantics:

  * /SkinGUIOptions/Skin renders inside amulegui itself, so a local
    fallback to "- default -" is the right behaviour (existing
    branch). Skin files genuinely can't be loaded from a remote host.

  * /WebServer/Template is consumed entirely by amuleweb, which may
    well be on a different filesystem from amulegui. The GUI has no
    business overwriting a value it doesn't recognise.

For the template case, append the unrecognised non-empty value to the
dropdown and select it, so TransferFromWindow reads back the same
string and the save round-trip becomes idempotent. Skin behaviour is
unchanged.

Verified on macOS arm64: monolithic amule rebuilds clean.

Reported by ngosang.
@got3nks got3nks mentioned this pull request May 15, 2026
Closed
@mrjimenez
mrjimenez merged commit bc68657 into amule-project:master May 15, 2026
12 checks passed
@got3nks
got3nks deleted the fix/prefs-skin-template-preserve branch May 15, 2026 18:19
mrjimenez pushed a commit to mrjimenez/amule that referenced this pull request Jul 27, 2026
@got3nks
fix(gui): batch the download list when downloading many search results (
123771b 
amule-project#621)

Downloading a large multi-selection of search results into a big queue
froze the monolithic GUI, the same way amule-project#615 did over the remote GUI --
just on a different, un-batched path.

CSearchListCtrl::DownloadSelected() loops over the selection calling
Search_Add_Download per file. In the monolithic build that notification
runs synchronously on the main thread, so each file's AddFile() fires a
per-item SortList() inline -- a full re-sort of the whole download list
on every insert, O(n^2) on a large queue.

Wrap the selection loop in the download list's BeginBatchUpdate() /
EndBatchUpdate() so the burst collapses into a single sort + repaint,
mirroring the remote GUI's poll path (amule-project#620). Monolithic-only: the remote
GUI's adds arrive later via the download-queue poll, which already
batches, so the code is gated behind #ifndef CLIENT_GUI.
mrjimenez pushed a commit to mrjimenez/amule that referenced this pull request Jul 27, 2026
@got3nks
fix(remote-gui): batch the shared-files list on every poll, not just …
4f5f24f 
…reconnect (amule-project#622)

Companion to amule-project#620/amule-project#621. Adding a burst of freshly-shared files over the
remote GUI -- many downloads completing daemon-side, or a shared folder
added/rescanned on the daemon -- froze the GUI on a large share. amule-project#620
fixed the equivalent download-list case but deliberately left the
shared-files ctrl batched reconnect-only.

CKnownFilesRem::ProcessUpdate() adds each new shared file via
CSharedFilesCtrl::ShowFile() -> AddItemData(), which on this virtual list
does a sorted insert AND rebuilds the entire row index (RebuildRowIndex)
on every call -- O(n) per insert, O(n^2) for a burst on an n-file share.
Unlike the download list, a plain Freeze/deferred-sort batch wouldn't
help, because the per-insert cost is the row-index rebuild, not a sort.

Give the shared-files ctrl the same append-path batching the download
list already uses: during a batch, ShowFile() appends with
AppendItemDataNow() (O(1), and it keeps the row index and item count live
so the interleaved reconcile prune and in-place UpdateItem() stay
correct), and the single SortList() is deferred to EndBatchUpdate(doSort).
The non-batch single-add path is unchanged. ProcessUpdate() now batches
the shared list on every non-initial poll and sorts once at the end only
if the poll added a file (sharedListGrew).

Monolithic is unaffected: its bulk shared paths funnel through the
batched Reload() -> ShowFileList(), and the dir-watcher coalesces bursts
into a single Reload(). Verified by building amule and amulegui.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Changes in preferences in Amule GUI overwrites some preferences

2 participants

@got3nks @mrjimenez