amule: aggregate AddLink failures into one popup (extends #310) - #577
Merged
mrjimenez merged 1 commit intoMay 11, 2026
Merged
Conversation
…ct#310) Adds a batch CDownloadQueue::AddLinks / CDownQueueRem::AddLinks abstraction and routes the GUI's batch callers (Fast eD2k paste box, .emulecollection import) through it. Centralises the popup aggregation logic and fixes both monolithic *and* amulegui in one move. Background: PR amule-project#557 made amulegui's CAddLinkHandler pop a wxMessageBox so an EC_OP_FAILED reply from EC_OP_ADD_LINK surfaces to the user instead of being silently dropped (amule-project#310). The same regression existed in the monolithic amule binary: CDownloadQueue::AddLink logs the failure via AddLogLineC and returns false, but no UI feedback ever reaches the user. The naive fix -- popping a wxMessageBox inside AddLink -- produces N dialogs when the user pastes N invalid lines into the Fast eD2k box. PR amule-project#551 had already taught amuled's EC handler to accept N tags in one EC_OP_ADD_LINK packet and emit a single aggregated EC_OP_FAILED response, but amulegui's CDownQueueRem::AddLink was still looping client-side and sending one packet per link, so CAddLinkHandler still fired once per failure. Change: 1. CDownloadQueue::AddLinks(wxArrayString, cat) Calls AddLink in a loop, counts failures, dispatches a single wxPLURAL'd ShowAlert at the end. ShowAlert is polymorphic (wxMessageBox under amule, log-only under amuled), so the same code is safe to compile into the daemon. 2. CDownQueueRem::AddLinks(wxArrayString, cat) Packs the entire batch into one EC_OP_ADD_LINK packet (one child tag per link, EC_TAG_PARTFILE_CAT carrying the category). Daemon aggregates per PR amule-project#551; CAddLinkHandler fires exactly once with the existing catalog string ("%d of %d links failed..."). 3. The two uniform-cat batch callers now call AddLinks instead of looping AddLink: - CamuleDlg::OnBnClickedFast (Fast eD2k paste box) - CSharedFilesCtrl::OnAddCollection (.emulecollection import) 4. CamuleAppCommon::AddLinksFromFile (ED2KLinks-in-config-dir poller) keeps its own per-line counter + AddLink-in-loop because each line may carry its own ":CAT" suffix, which doesn't fit the single-cat AddLinks signature. Rare path, rare feature -- left alone. CDownloadQueue::AddLink itself is untouched; it still logs the protocol-specific reason per failure, which the aggregated popup intentionally points to ("see log for details") rather than duplicating inline.
got3nks
force-pushed
the
fix/addlink-aggregate-popup
branch
from
May 11, 2026 09:44
4b0826a to
9da6339
Compare
got3nks
added a commit
to got3nks/amule
that referenced
this pull request
Jul 24, 2026
…ip (amule-project#577) The "Auto-update ipfilter at startup" toggle (IPFilterAutoLoad) ships on by default, but the update is gated on a non-empty IPFilterURL, which defaulted to "" -- so the default-on toggle did nothing. Set the default URL to https://upd.emule-security.org/ipfilter.zip, the same source already used for the server list (server.met) and Kad nodes (nodes.dat). Fresh installs now conditionally fetch the filter on startup (the download uses If-Modified-Since, so an unchanged file returns 304 and is skipped). Existing configs that already stored an empty IPFilterURL keep their value.
ngosang
pushed a commit
to ngosang/amule
that referenced
this pull request
Jul 24, 2026
) Ship the emule-security ipfilter.zip URL as the default (from amule-project#577) but leave the auto-update toggle off by default, so new users opt in rather than silently loading a filter at startup. emule-security's list blocks whole hosting ranges, which flags many VPN exit IPs; a default-on fetch would block those users before they ever open the preferences. Existing configs keep their stored IPFilterAutoLoad value; this only changes the shipped default for new configs. The URL default is unchanged, so ticking the box works out of the box.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
PR #557 fixed the silent failure in amulegui (issue #310): pasting a malformed link no longer disappears into the void — the user gets a dialog. The same regression still existed in the monolithic amule binary:
CDownloadQueue::AddLinklogs the failure viaAddLogLineCand returnsfalse, but no UI feedback ever reaches the user.The naive fix — popping a
wxMessageBoxfrom insideAddLink— would produce N dialogs when the user pastes N invalid lines into the Fast eD2k box. And amulegui has the same N-popup problem from a different angle: PR #551 already taught amuled's EC handler to accept N child tags in oneEC_OP_ADD_LINKpacket and emit a single aggregatedEC_OP_FAILEDresponse, butCDownQueueRem::AddLinkwas still looping client-side and sending one packet per link.What
Introduce a batch
AddLinks(wxArrayString, uint8 cat)abstraction on both queue types, so the GUI's batch callers go through one symmetric API and each app aggregates in the way that fits its transport:CDownloadQueue::AddLinks(monolithic + daemon): loopsAddLink, counts failures, dispatches onewxPLURAL'd dialog throughtheApp->ShowAlert.ShowAlertis polymorphic (wxMessageBox under amule, log-only under amuled) so the same code is safe to compile into the daemon andamuledkeeps its quiet-batch behaviour.CDownQueueRem::AddLinks(amulegui): packs the entire batch into a singleEC_OP_ADD_LINKpacket (one child tag per link,EC_TAG_PARTFILE_CATcarrying the category). The daemon's existing EC: aggregate per-link result into a single ADD_LINK response #551 aggregation produces one reply;CAddLinkHandlerfires exactly once with the existing catalog string"%d of %d links failed (invalid or already on list).".Two GUI batch callers switched from
AddLink-in-loop to oneAddLinkscall:CamuleDlg::OnBnClickedFast(Fast eD2k paste box)CSharedFilesCtrl::OnAddCollection(.emulecollectionimport)CamuleAppCommon::AddLinksFromFile(theED2KLinks-in-config-dir poller) keeps a per-line counter +AddLink-in-loop because each line can carry its own:CATsuffix, which doesn't fit a single-category batch signature. Rare path, rare feature — left alone.CDownloadQueue::AddLinkitself is untouched: it still logs the protocol-specific reason per failure, which the aggregated popup intentionally points to (see log for details) rather than duplicating inline.Test
2.3.3-353-g9da63394a): amule, amulegui, amuled, amulecmd, amuleweb all compile clean.ED2KLinksfile with 2 valid + 2 invalid links into~/Library/Application Support/aMule/: one dialog appeared reading "Could not add 2 links from ED2KLinks file (see log for details).", the two invalid lines logged with their specific reasons, the two valid lines added to the queue.AddLinksmethod on both binaries (compile-tested; runtime smoke test still recommended on amulegui talking to amuled before merge).