Fall back instead of rendering broken roster images (#389) - #390
Merged
Merged
Conversation
Every roster image URL was built straight from a DB value with no existence check, so a plugin missing one asset rendered a broken image. That is the shared root cause behind the per-plugin icon tickets (eq#7, eq2#7, ffxiv#4, gw2#7, lineage2#3/#7, lotro#3, swtor#3) and the only fix that scales to bbguildlineage2's 110 missing roster portraits. - class_image_candidates(): roster art → detail icon → the game's <prefix>_unknown icon in either dir - character_image_candidates(): the class/race image, then the same-directory unknown icon - resolve_game_image(): first candidate that exists on disk, re-anchoring the web path at its ext/ segment the way resolve_portrait_with_fallback() already does; '' when none exist - roster_center.html guards all four <img> sites so an unresolved image omits the tag rather than emitting an empty src - DB-sourced names are reduced to basename(), so a crafted imagename cannot walk out of the images directory 13 tests cover the candidate ordering, the traversal strip, and each fallback step against core's own images/custom_*.png. Fixes #389 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
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
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.
Closes #389.
Every roster image URL was built straight from a DB value with no existence check, so a game plugin missing one asset rendered a broken image. That is the shared root cause behind the per-plugin icon tickets (bbguildeq#7, bbguildeq2#7, bbguildffxiv#4, bbguildgw2#7, bbguildlineage2#3/#7, bbguildlotro#3, bbguildswtor#3), and the only fix that scales to bbguildlineage2's 110 missing roster portraits.
What changed
class_image_candidates()— roster art → detail icon → the game's<prefix>_unknownicon in either directorycharacter_image_candidates()— the character's class/race image, then the same-directory unknown iconresolve_game_image()— first candidate that exists on disk, re-anchoring the web path at itsext/segment the wayresolve_portrait_with_fallback()already does;''when none existroster_center.htmlguards all four<img>sites, so an unresolved image omits the tag instead of emitting an emptysrcbasename(), so a craftedimagenamecannot walk out of the images directoryTests
13 new tests in
tests/portal/roster_image_fallback_test.phpcover candidate ordering, the traversal strip, and each fallback step. They resolve against core's ownimages/{roster_classes,class_images}/custom_*.png— no game plugin involved, so they hold in CI where none are installed.Full core unit suite: 115 tests, 306 assertions, green.
🤖 Generated with Claude Code