Page MenuHomeWildfire Games

change isUndeletable to getDeletionPolicy and hide deletion option for animals
Needs ReviewPublic

Authored by abian on Aug 11 2023, 5:13 PM.

Details

Reviewers
Freagarach
Trac Tickets
#6852
Summary

isUndeletable is now getDeletionPolicy, and it's called twice instead of three times. This function is used to hide the (previously grayed-out) deletion icon for player-owned animals, since (a) Gaia-owned animals, which are the majority, don't display this icon either; and (b) already dead animals previously displayed this icon with an inconsistent tooltip ("The entity has to be killed before it can be gathered from"), and hiding the icon is the simplest and least confusing option for the player. Periods (.) have been added to full sentences in tooltips (I will help copy old translations).

Test Plan

Tested in the game. Animals don't have a deletion icon, regardless of who owns them. The option to delete previously deletable entities remains unchanged. Undeletable entities owned by the player and those entities owned by the player with less than 50% of capture points have the grayed-out icon with explanatory tooltip(s), as before. If deletable and non-deletable entities are selected, the deletion icon is shown enabled, as before.

Event Timeline

abian created this revision.Aug 11 2023, 5:13 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/8868/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/7197/display/redirect

Freagarach published this revision for review.Aug 14 2023, 10:35 AM

(If a revision takes too long to get out of 'draft' status, one can manually 'request review' using the dropdown at the bottom of the page 'Add Action'.)

I haven't reviewed the logic yet, I'll try that later.

/home/abian/0ad/binaries/data/mods/public/gui/session/unit_actions.js
1417 ↗(On Diff #22128)
1419 ↗(On Diff #22128)

One can inline the above query.

1423 ↗(On Diff #22128)

(Inline)

1437 ↗(On Diff #22128)

You don't need an else after a return. :)

abian updated this revision to Diff 22134.Aug 14 2023, 2:51 PM

Thanks! Done.

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/8872/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/7201/display/redirect

phosit added a subscriber: phosit.Aug 14 2023, 7:50 PM

You removed quotation marks in object-keys (e.g. "tooltip": to tooltip:) is there consent on that?

Could you document what a policy is. Or write a comment what getDeletionPolicy returns.
The policy should have a isDeleteable method. Because the client-code should not have to know that if there is no undeletableReason it is deleteable.

abian updated this revision to Diff 22137.Aug 14 2023, 10:25 PM

Useful feedback as well. Thanks!

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/vs2015-differential/8292/display/redirect

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/7203/display/redirect

Freagarach added inline comments.Aug 16 2023, 7:52 AM
binaries/data/mods/public/gui/session/unit_actions.js
1433

(With inline I meant this. We do want to keep the returns on a new line.)