Before Alpha 22, only the persian palace had a resource trickle. With this release, wonders and the macedonian catafalque (Philip V) also have it, so we should inform the player.
Also prevents empty loot tooltip in case of an entity having a loot component but no loot numbers.
Details
Details
- Reviewers
fatherbushido - Commits
- rP19387: Resource Trickle tooltip.
- Trac Tickets
- #4118
Yo dawg, I heard you like sprintf. (So I put a sprintf into the sprintf and so on)
Open the tech tree and read the tooltip of the wonder and persian palace.
Start a capture the relic match and look for Philip V and the tooltip.
Remove ResourceGather disabled="" from special_catafalque.xml and check that there is no "Loot:" string.
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Build has FAILED
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (305 tests).ERROR: JavaScript error: simulation/components/GuiInterface.js line 587 ReferenceError: IID_ResourceTrickle is not defined GuiInterface.prototype.GetExtendedEntityState@simulation/components/GuiInterface.js:587:6 @simulation/components/tests/test_GuiInterface.js:602:25 In TestComponentScripts::test_scripts: /mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_GuiInterface.js" /mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content) ............................................................................................................................................................................................................................................................................................................... Failed 1 and Skipped 0 of 305 tests Success rate: 99%
Link to build: http://jw:8080/job/phabricator/698/
See console output for more information: http://jw:8080/job/phabricator/698/console
Comment Actions
Code:
Checked all the not sprintf stuff.
Test:
unit with Loot, without ResourceGatherer -> loot tooltip
unit without Loot, with ResourceGatherer -> loot tooltip when carrying
unit without Loot and withtout ResourceGatherer -> no tooltip
ResourceTrickle tooltip ok, take account of tech and aura modification too (see Templates.js)
The tooltip seems fine (but my opinion about that is not really valuable.
Attack rate tooltip not broken.
Checked stucttree and session.
| binaries/data/mods/public/globalscripts/Templates.js | ||
|---|---|---|
| 257 ↗ | (On Diff #1114) | missing space |
| binaries/data/mods/public/gui/common/tooltips.js | ||
| 416 ↗ | (On Diff #1114) | shouldn't here be a round/tofixed? (imagine a mod adding a tech for this) I guess stupid people adding a 0 rate should be punished with an ugly tooltip. Actually a tech could enable it, probably need a fix. |
| binaries/data/mods/public/gui/structree/draw.js | ||
| 4–19 ↗ | (On Diff #1114) | The order of these list could use some syncing, suggesting alphabetical order or same order as they are used. But out of scope. |
| binaries/data/mods/public/gui/common/tooltips.js | ||
|---|---|---|
| 416 ↗ | (On Diff #1114) | Sounds like, it's the best place to do it. |
Comment Actions
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (305 tests).................................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/701/ for more details.
| binaries/data/mods/public/gui/common/tooltips.js | ||
|---|---|---|
| 416 ↗ | (On Diff #1114) | The annoying thing is if we have for example 0.001 each 1000 ms, I don't really get how to round that. |
Comment Actions
Thanks for the reviews! I've added that space and filtered 0 resource entries.
| binaries/data/mods/public/gui/common/tooltips.js | ||
|---|---|---|
| 416 ↗ | (On Diff #1114) | We wanted to keep resources integer, so there shouldn't be a template value providing a float in the first place, nor an aura effecting the resource amount if it's not an integer number too (like 2x bonus). For the same reason we changed the interval for the relic to 1250. Having zeroes in there sounds more probable and I've had to add commits for special cases often enough to add a check preemptively now. |
| binaries/data/mods/public/gui/structree/draw.js | ||
| 4–19 ↗ | (On Diff #1114) | The order here in the order seen in the tooltips. |