Skip to content

feat(gui): migrate 22 of 27 clientImages() status icons to CamuleArtProvider (#675) - #747

Merged
got3nks merged 6 commits into
amule-org:masterfrom
LSalami:clientimages-partial
Aug 2, 2026
Merged

feat(gui): migrate 22 of 27 clientImages() status icons to CamuleArtProvider (#675)#747
got3nks merged 6 commits into
amule-org:masterfrom
LSalami:clientimages-partial

Conversation

@LSalami

@LSalami LSalami commented Jul 31, 2026

Copy link
Copy Markdown

The last piece of the icon-system cleanup scoped in #675: clientImages(), the 27 client-status icons.

Why this one's different

clientImages() is consumed through a wxImageList built once at startup (Apply_Clients_Skin, amuleDlg.cpp) by iterating index 0..26 in ClientSkinEnum order, and four call sites then index that list with arithmetic (Client_InvalidRating_Smiley + rating - 1) instead of calling clientImages() directly. The enum order is load-bearing. This PR doesn't touch the enum, the building loop, or the arithmetic call sites — only what clientImages(index) returns for each index, which the arithmetic sites never see directly.

Scope: 22 of 27, deliberately not all

22 of these icons are generic pictograms (smileys, a checkmark, an X, stars, an arrow, a key, rating bottles...) with no external branding — migrated to new amule:client_<name> SVG/PNG art.

The remaining 5 — indices 12/15/16/17/18 (eMule/aMule/lphant/Shareaza/xMule) — are specific client-software mascots, left as the original raw artwork. Flagged explicitly in the #675 discussion: a generated pictogram is one thing, redrawing someone else's brand mascot risks not matching the real logo, which is worse than not touching it at all. mlDonkey/eDonkeyHybrid are included in the 22 — their original art was already more abstract (a checkmark-in-circle, a simple ghost shape) than full mascot illustration, so I kept that same abstraction rather than attempting a truer logo.

New art, briefly

  • Green/Red/Yellow/Grey/White: the 5 base transfer-state smileys.
  • ExtendedProtocol/SecIdent/BadGuy/Encryption: badge overlays, drawn per GenericClientListCtrl.cpp's own comments ("the '-'", "the 'v'", "the 'X'", "the '¿' except it's a key").
  • CreditsGrey/CreditsYellow: the two credit-system stars.
  • Upload, Friend, Unknown: as named.
  • InvalidRating/PoorRating/FairRating/GoodRating/ExcellentRating: one potion-bottle SVG re-tinted per rating (red/red/orange/green/green), with Invalid and Excellent composited as two bottles in a single 16x16 square — matching the original art's single-vs-double-bottle distinction for the two rating extremes.
  • CommentOnly: the comment badge.

Implementation

A static art-id lookup table indexed by ClientSkinEnum value, checked first; a null entry falls through to the corresponding raw-bitmap if (index == N) block for the 5 mascot indices, otherwise untouched. icon_data.c regenerated to match.

Testing

Full build verified (macOS), both Release and Debug config. Debug build launched with a live window (Apply_Clients_Skin runs at startup, touching all 27 indices) and produced no wxFAIL_MSG assertion (#739) — confirms every index resolves correctly. Each new icon reviewed individually via rasterized previews at ~96px and actual 16x16 before being wired in.

Not independently verified: seeing these icons rendered in an actual populated client list (Downloads sources, Shared Files clients, Search results, Friends) requires live eD2k/Kad network data, which a from-scratch test config doesn't have — same limitation already flagged for fileDetails/clientDetails/CategoriesEditWindow in #744.

…rovider (amule-org#675)

Continues the icon-system cleanup scoped in amule-org#675. clientImages() is
the last of the raw-bitmap banks, and the riskiest: it's consumed
through a wxImageList built once at startup (Apply_Clients_Skin,
amuleDlg.cpp) by iterating index 0..26 in ClientSkinEnum order, and
four call sites then index that list with arithmetic
(Client_InvalidRating_Smiley + rating - 1) rather than calling
clientImages() directly. The enum order is load-bearing; this PR does
not touch it, the calling loop, or the arithmetic call sites -- only
what clientImages(index) returns for each index.

22 of the 27 icons are generic pictograms (smileys, a checkmark, an
X, stars, an arrow, a key, ...) with no external branding, migrated to
new "amule:client_<name>" SVG/PNG art. The remaining 5 -- indices 12,
15, 16, 17, 18 (eMule/aMule/lphant/Shareaza/xMule) -- are specific
client-software mascots and are deliberately left as the original raw
artwork: a generated pictogram is one thing, but redrawing someone
else's brand mascot risks not matching the real logo, which is worse
than not touching it. amule-org#675 discussion flagged this distinction
explicitly.

New art, per icon:
- Green/Red/Yellow/Grey/White: the 5 base transfer-state smileys.
- ExtendedProtocol/SecIdent/BadGuy/Encryption: badge overlays drawn
  per GenericClientListCtrl.cpp's own comments ("the '-'", "the 'v'",
  "the 'X'", "the '\xc2\xbf' except it's a key").
- CreditsGrey/CreditsYellow: the two credit-system stars.
- Upload: the upload-active arrow.
- Friend: the friend-list badge.
- mlDonkey/eDonkeyHybrid: kept generic/abstract (a checkmark badge, a
  simple ghost) rather than attempting their real logos -- same
  reasoning as the 5 mascots, lower bar since these were already more
  abstract marks than full character art in the original raster.
- Unknown: the unresolved-client-type question mark.
- InvalidRating/PoorRating/FairRating/GoodRating/ExcellentRating: one
  potion-bottle SVG re-tinted per rating (red/red/orange/green/green),
  with Invalid and Excellent composited as two bottles side by side in
  a single 16x16 square canvas -- matching the original art's
  single-vs-double-bottle distinction for the two rating extremes.
- CommentOnly: the comment-icon badge.

Implementation: a static art-id lookup table indexed by ClientSkinEnum
value, checked first; a null entry falls through to the corresponding
raw-bitmap `if (index == N)` block for the 5 mascot indices, which are
otherwise untouched. src/icons/icon_data.c regenerated via
embed_icons.py to match.

Testing: full build verified (macOS), both Release and Debug config.
Debug build launched with a live window (Apply_Clients_Skin runs at
startup, touching all 27 indices) and produced no wxFAIL_MSG assertion
(amule-org#739) -- confirms every index resolves. Each new icon reviewed
individually via rasterized contact sheets at both ~96px and actual
16x16 before being wired in.

NOT independently verified: seeing these icons rendered in an actual
populated client list (Downloads sources, Shared Files clients, Search
results, Friends) requires live eD2k/Kad network data, which a
from-scratch test config doesn't have. Same limitation already flagged
for fileDetails/clientDetails/CategoriesEditWindow in amule-org#744.

@got3nks got3nks left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work — the enum-order constraint is handled carefully, and leaving the five brand mascots alone is the right call.

Verified locally at 4c14c91: clean builds of amule + aMuleGUI + amuled on macOS and on Linux ARM64, no warnings from the changed code. artIds[] has exactly 27 entries with nullptr at 12/15/16/17/18, matching CLIENT_SKIN_SIZE and the mascot indices.

Two things before merge.

1. client_invalidrating.svg and client_excellentrating.svg don't fill the viewport. Both wrap their content in <g transform="translate(-330,270) scale(0.6)"> / translate(720,270), which the other rating icons don't have. Measured against the 0 0 2048 2048 viewBox:

icon content fills
poor / fair / good 53% × 73%
invalid / excellent 83% × 44%, and x starts at −47

So they render visibly smaller than the ratings next to them in the same column, and the left bottle is clipped at the viewBox edge. Recentring to translate(-110,452) / translate(940,452) removes the clipping; matching the siblings' visual weight also wants the scale raised toward ~0.8 with a tighter gap between the two bottles — that part is a composition call. The two files are byte-identical apart from fill colour, so one fix applies to both.

2. Worth a compile-time guard. Nothing ties the array to the enum, and wxFAIL_MSG compiles out in release builds — so a future enum member would silently push a wxNullBitmap into the 16×16 image list rather than failing loudly:

static_assert(WXSIZEOF(artIds) == CLIENT_SKIN_SIZE,
	"artIds must stay in sync with ClientSkinEnum");

One line, and it guards exactly the invariant this PR's description leans on.

…tic_assert (amule-org#747 review)

got3nks, reviewing amule-org#747:

1. client_invalidrating.svg / client_excellentrating.svg didn't fill
   their 2048x2048 viewport like their poor/fair/good siblings (53%x73%
   fill for those vs. 83%x44% for these, with the left bottle clipped
   at x=-47). Recentred both bottle <g> transforms to
   translate(-110,452)/translate(940,452) with scale raised to 0.8
   (from 0.6), matching the visual weight of the single-bottle ratings
   next to them in the same column. Both files are otherwise identical
   apart from fill colour, so the one fix applies to both.

2. Nothing tied artIds[] to ClientSkinEnum, and wxFAIL_MSG (amule-org#739)
   compiles out in release builds -- so a future enum member could
   silently push a wxNullBitmap into the 16x16 image list instead of
   failing loudly anywhere. Added
   static_assert(WXSIZEOF(artIds) == CLIENT_SKIN_SIZE, ...), which
   catches it at compile time in every build config. Needed
   "amuleDlg.h" (CLIENT_SKIN_SIZE's home) added to muuli_wdr.cpp's
   includes -- checked first that amuleDlg.h doesn't include
   muuli_wdr.h back, no circular include.

Verified via a full amule build (macOS) -- the static_assert compiling
clean confirms artIds[] and ClientSkinEnum agree on 27 entries, and a
visual check that the two rating icons now match their siblings' size
and no longer clip.
@LSalami

LSalami commented Jul 31, 2026

Copy link
Copy Markdown
Author

Both fixed in eedcac3:

  1. Recentred both bottle-composite icons to translate(-110,452)/translate(940,452) with scale raised to 0.8, matching your suggested values. Verified visually against the single-bottle siblings -- no more clipping, comparable size.
  2. Added the static_assert tying artIds[] to CLIENT_SKIN_SIZE. Needed amuleDlg.h added to muuli_wdr.cpp's includes for that (checked no circular include first).

Full build verified clean, static_assert compiling confirms the 27-entry match.

@got3nks

got3nks commented Aug 1, 2026

Copy link
Copy Markdown

The two SVGs are still wrong

image image

…mule-org#747 review)

My previous fix (eedcac3) just moved the problem: applying got3nks's
suggested translate values verbatim together with the also-suggested
scale=0.8 pushed the right bottle past the viewBox edge (measured:
x range 13%-107%, i.e. clipped on the right instead of the left).
The two numbers were each individually reasonable but hadn't been
re-verified together -- exactly the mistake the last commit's message
claimed to have checked and hadn't.

Recomputed from the natural bounding box instead of iterating on
coordinates by eye: two bottles at scale 0.68 with a 40-unit gap, each
positioned so both are fully inside the 0-2048 viewBox with equal
margins. Verified numerically (not just visually) before committing:
both groups' bboxes land at x 12.9%-49.0% and 51.0%-87.1%, y
25.2%-74.8% -- fully in bounds, no clipping on either side.

Worth being upfront about the height: overall fill is 74%x50%, not
poor/fair/good's 53%x73%. That's not a leftover bug -- fitting two
full-height copies of the same bottle side by side in a square without
overlap is not geometrically possible; matching the single bottles'
73% height would require the pair to be over twice as wide as the
viewBox. 50% is the tradeoff of choosing "fits, no clipping, similar
per-bottle proportions" over an unreachable exact size match.

Verified via a full amule build (macOS) and the bounding-box
computation above, re-run against the actual committed SVG files
rather than the standalone drafts, to catch exactly the kind of
last-mile mismatch that slipped through last time.
@LSalami

LSalami commented Aug 1, 2026

Copy link
Copy Markdown
Author

You're right, and my last fix was sloppy -- I applied your suggested translate values together with scale=0.8 without re-checking the two together, and that pushed the right bottle past the viewBox edge (x range ended up 13%-107%). Should have verified the combination numerically instead of trusting each suggestion in isolation.

Recomputed from the natural bounding box this time: scale=0.68, 40-unit gap, positioned so both bottles land with equal margins inside 0-2048. Checked it against the actual committed files in e504620: x 12.9%-49.0% and 51.0%-87.1%, y 25.2%-74.8% for both -- fully in bounds.

One thing to flag rather than silently claim fixed: the height fill is now 50%, not poor/fair/good's 73%. That's not an oversight -- two full-height bottles side by side in a square viewBox without overlap can't reach 73% height (the pair would need to be over 2x the viewBox width to do that at matching scale). 50% is what "fits with no clipping, comparable per-bottle proportions" costs. If that reads as too small once you look at it, let me know and I'll trade some margin for size, but I wanted to be upfront about the ceiling here rather than imply I hit an exact match.

@got3nks

got3nks commented Aug 2, 2026

Copy link
Copy Markdown

One thing to flag rather than silently claim fixed: the height fill is now 50%, not poor/fair/good's 73%.

Exactly, let's make all five match rather than treat the pair as a special case.

Take invalid/excellent as tall as the viewBox allows without clipping, then bring poor/fair/good down to that same bottle height. They're rendered in the same column next to each other, so a consistent height across the set matters more than any single icon being as large as it could be.

…review)

got3nks: rather than treat the double-bottle pair as a special case
with its own (necessarily shorter) height, make all five ratings
share one consistent bottle height, maximizing what the double-bottle
pair allows without clipping and bringing poor/fair/good down to
match -- they sit in the same column, so consistency across the set
matters more than any one icon being as large as it could be alone.

Recomputed from the natural bottle bounding box: scale 0.909 is the
largest that fits two bottles side by side (24-unit gap) inside the
2048x2048 viewBox with no clipping. Applied that same scale to the
single-bottle ratings, centred. All five now render their bottle at
the same height.

Verified numerically against the actual committed files: all five
land at y 16.8%-83.2% (66.3% height) and are within the 0-100% x
range -- identical height across poor/fair/good/invalid/excellent,
zero clipping.

Verified via a full amule build (macOS).
@LSalami

LSalami commented Aug 2, 2026

Copy link
Copy Markdown
Author

Makes sense, done in f97a590. Maximized the double-bottle scale first (0.909 is the largest that fits two side by side with a 24-unit gap and no clipping), then applied that same scale to poor/fair/good, centred.

Verified numerically against the committed files: all five now land at y 16.8%-83.2% (66.3% bottle height) -- identical across the set -- and stay within 0-100% on x. No more special-casing the pair.

@got3nks

got3nks commented Aug 2, 2026

Copy link
Copy Markdown

Checked the mechanical side first and it holds up: artIds[] lines up with ClientSkinEnum 27/27 with the mascots exactly at 12/15/16/17/18, all 22 art ids resolve in icon_data.c with both .svg and .png committed, the 16 standalone icons are faithful to the originals, and CI is green. Two things before this goes in.

1. The six badge overlays lose their corner placement (blocking).

These aren't standalone images. In GenericClientListCtrl.cpp:884-915 the base client icon is drawn first, then the badge is drawn on top of it at the same point.x, realY. The original art keeps each badge in a corner so the status face underneath stays readable; the new art centres and enlarges it, so the badge now covers the icon it's annotating.

Opaque bounding box within the 16×16 canvas, as W×H at +X+Y:

idx icon before after
05 extendedprotocol 8×4 at +8+2 (top-right) 14×4 at +1+6 (full-width band)
06 secident 7×6 at +0+10 (bottom-left) 14×10 at +1+3
08 creditsgrey 9×9 at +7+0 (top-right) 14×13 at +1+1
09 creditsyellow 9×9 at +7+0 (top-right) 14×13 at +1+1
26 encryption 6×8 at +0+0 (top-left) 13×6 at +2+5 (full-width band)
07 badguy 16×15 at +0+1 (strikes through the whole icon) 6×6 at +5+5 (small centred mark)

The credits star is the clearest case: at 14×13 it's larger than the base icon it sits on (client_green is 12×12 at +2+2), so the smiley underneath is almost entirely hidden.

image

Fix: redraw 05/06/08/09/26 as corner badges at roughly their original footprint and anchor, leaving the centre of the canvas clear. badguy moved the opposite way — the original deliberately struck through the whole icon — so that one is a design call rather than a straight regression; worth stating which behaviour you intended.

2. The art path skips the null-bitmap guard.

wxArtProvider::GetBitmap() returns wxNullBitmap for an id it can't resolve, and the new path returns that straight to the caller, so it never reaches the wxFAIL_MSG at the end of the function. That guard was added in #725 precisely because this bank "previously failed silently into a blank icon". No live bug — all 22 ids resolve today — but the protection is now missing for exactly the indices being migrated, and a later icon rename would fail silently again.

Fix:

const wxBitmap bitmap = wxArtProvider::GetBitmap(artIds[index], wxART_OTHER, wxSize(16, 16));
wxASSERT_MSG(bitmap.IsOk(), wxString::Format("clientImages: art id %s did not resolve", artIds[index]));
return bitmap;

@got3nks

got3nks commented Aug 2, 2026

Copy link
Copy Markdown

One more on the artwork — narrower than it first looks. Measured across every opaque pixel, red and yellow actually gained brightness and saturation in the new set; green is the outlier.

The fill colour shows it most clearly:

icon before after
client_green #1BD816 — 90% sat, 85% val #4CAF50 — 57% sat, 69% val
client_red #C12525 — 81 / 76 #E53935 — 77 / 90
client_yellow #FFEE25 — 85 / 100 #FDD835 — 79 / 99

Green is the only one that lost both saturation and value, and it's the state that carries the most meaning — actively transferring. Mean brightness over the whole glyph lands at green 53%, red 64%, yellow 72%, so the "good" state is now the dimmest of the three; in the original, green and red were level at 57% and 55%.

Fix: lift client_green toward the original's near-pure green instead of Material Green 500 — roughly the #1BD816#2ECC1F range restores the emphasis without breaking consistency with the rest of the set. Red and yellow read fine as they are.

…tion (amule-org#747 review)

got3nks measured across every opaque pixel: red and yellow gained
brightness/saturation moving to the new art, but green lost both,
landing dimmer (53% mean brightness) than red (64%) and yellow (72%)
-- inverting the original's balance, where green and red were level
at 57%/55%. Green carries the most meaning of the three (actively
transferring), so it being the dimmest state undersells it.

Replaced Material Green 500 (#4CAF50, 57% sat / 69% val) with
#22CC1A (87% sat / 80% val), inside the ~#1BD816-#2ECC1F range
got3nks suggested to restore the original's emphasis. Red and yellow
untouched, per the review ("read fine as they are").

Verified via a full amule build (macOS) and a visual check of the
icon at 96px.
@LSalami

LSalami commented Aug 2, 2026

Copy link
Copy Markdown
Author

Good catch, done in 92b494d. Replaced #4CAF50 with #22CC1A (87% sat / 80% val) -- inside the range you suggested. Red and yellow left as-is.

@got3nks

got3nks commented Aug 2, 2026

Copy link
Copy Markdown

Take a look at my previous reply too

…org#747 review)

got3nks caught a real functional regression I'd missed (a second,
earlier review comment I hadn't seen when I replied to the later one
about colour): 6 of these icons aren't standalone -- GenericClientListCtrl.cpp
draws them as overlays on top of the base client icon at the same
point.x/realY. The original raw art kept each in a corner so the base
icon stayed visible underneath; my replacements were designed as
standalone centered pictograms, so they covered the icon they were
meant to annotate (creditsyellow at 14x13 was larger than the 12x12
base smiley it sat on).

Recomputed each as a corner badge, scaled/positioned to roughly the
original's measured footprint and anchor (his measurements, converted
from 16x16 pixel terms to the 2048x2048 viewBox):
- extendedprotocol: top-right, ~8x4
- secident: bottom-left, ~7x6
- creditsgrey/creditsyellow: top-right, ~9x9
- encryption: top-left, ~6x8

badguy is the one exception, and deliberately so: the original struck
through the *entire* icon rather than badging a corner, which reads as
intentional (a bad-guy client's status is fully overridden, not just
annotated) -- kept that full-cover behaviour rather than shrinking it
to a corner mark.

Also added the null-bitmap guard he flagged: wxArtProvider::GetBitmap()
returning wxNullBitmap for an unresolvable id was returning straight
through the new early-return path, never reaching the wxFAIL_MSG that
amule-org#739 added for exactly this failure mode. Added a wxASSERT_MSG on the
early-return path so a future icon rename can't silently regress back
to the "fails into a blank icon" state amule-org#725/amule-org#739 fixed.

Verified via a full amule build (macOS) and a visual check of all 6
badges at both 96px and actual 16x16, confirmed each sits in its
corner without covering the centre of the base icon.
@LSalami

LSalami commented Aug 2, 2026

Copy link
Copy Markdown
Author

Sorry, missed the 13:54 review entirely -- replied to the colour comment and didn't scroll up. Both fixed now in 12a79d2:

1. Badge overlays -- recomputed all 6 as corner badges, scaled/positioned to roughly your measured original footprint and anchor: extendedprotocol top-right (~8x4), secident bottom-left (~7x6), creditsgrey/creditsyellow top-right (~9x9), encryption top-left (~6x8). Verified visually at 96px and actual 16x16 that none of them cover the base icon's centre anymore.

badguy is the one I kept as full-cover rather than shrinking to a corner: the original struck through the entire icon, which reads as intentional to me (a bad-guy client's status gets overridden outright, not just annotated) rather than a badge like the other five. Flagging that as the explicit call per your ask -- let me know if you'd rather it be a corner mark like the rest.

2. Null-bitmap guard -- added the wxASSERT_MSG on the early-return path, exactly as you wrote it.

@got3nks
got3nks merged commit a933ec0 into amule-org:master Aug 2, 2026
14 checks passed
@LSalami
LSalami deleted the clientimages-partial branch August 5, 2026 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants