As we now have linear and circular splash damage in the game, this should be shown in the tooltip.
Details
- Reviewers
elexis - Commits
- rP19229: Splash Damage shape tooltip
- Trac Tickets
- #4328
Check for Linear/Circular Splash Damage in:
- structree tooltip ingame/mainmenu
- trainging panel
- single selection details panel
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
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/356/ for more details.
binaries/data/mods/public/gui/common/tooltips.js | ||
---|---|---|
209 ↗ | (On Diff #522) | (perhaps missing spaces) |
binaries/data/mods/public/gui/common/tooltips.js | ||
---|---|---|
209 ↗ | (On Diff #522) | I am not able to review that part :/ |
binaries/data/mods/public/gui/common/tooltips.js | ||
---|---|---|
209 ↗ | (On Diff #522) | I should use a global object for {"Circular": translate("Circular"), "Linear": translate("Linear")} |
binaries/data/mods/public/gui/common/tooltips.js | ||
---|---|---|
209 ↗ | (On Diff #522) | We were wondering whether the array becomes constructed again with each call (onTick) (old leper argument), but spidermonkey seems to be smart enough to cache that. However having a global object means that modders can modify that array externally outside of this file (old sanderd17 argument). Besides I believe that {"Circular": translate("Circular"), "Linear": translate("Linear")}[splash.shape] } looks ugly. |
binaries/data/mods/public/gui/common/tooltips.js | ||
---|---|---|
207 ↗ | (On Diff #522) | Sometimes the colon is colorized in the file, other times it isn't. I wonder though why we need this new string when we could reuse the existing "%(label)s: %(value)s" |
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/361/ for more details.
binaries/data/mods/public/gui/common/tooltips.js | ||
---|---|---|
20 ↗ | (On Diff #534) | If the accepted argument is allowing modders to extend the attack types without inserting a line of code here, that shouldn't pretend to be unmodifiable (i.e. const -> var). |
213 ↗ | (On Diff #534) | Sorry for not having it posted before, but I wouldn't be surprised if there is some language that has issues with that string, |
Build is green
Updating workspaces. Build (release)... Build (debug)... Running release tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!
http://jw:8080/job/phabricator/362/ for more details.
Thanks for the patch, tested, works, addressed all remarks, yadda, yadda.
Wondering whether we should remove the friendly fire tooltip as we disabled all friendly fire.
Mods could still have it enabled... perhaps it would be the best to have a global boolean showFriendlyFireTooltip, so that we can change our minds without changing the code and allow modders to enable the tip from an external file?
Wondering whether we should remove the friendly fire tooltip as we disabled all friendly fire.
Mods could still have it enabled... perhaps it would be the best to have a global boolean showFriendlyFireTooltip, so that we can change our minds without changing the code and allow modders to enable the tip from an external file?
(displaying only when true)
(When the splash damage tooltip and it's friendly fire part were added in rP18477, I thought about showing the string in both cases so as to avoid confusion about the value of the property when the value is hidden. You might be right though that hiding it if it's disabled is more appropriate to our use case.)