Page MenuHomeWildfire Games

Wondervictory entity rename
ClosedPublic

Authored by bb on Jan 5 2019, 8:54 PM.

Details

Summary

Following rP22019/D1694 wonder victory needs a similar treatment

Test Plan

create an upgradable wonder, which can upgrade to another wonder (or his own template) and to a non-wonder and see that the timer is set correctly in all cases

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

bb created this revision.Jan 5 2019, 8:54 PM
Owners added a subscriber: Restricted Owners Package.Jan 5 2019, 8:54 PM
Vulcan added a subscriber: Vulcan.Jan 5 2019, 8:56 PM

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

Linter detected issues:
Executing section Source...
Executing section JS...
|    | [NORMAL] ESLintBear (indent):
|    | Expected indentation of 4 tabs but found 3.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/maps/scripts/WonderVictory.js
| 106| 106| 					"players": [player],
| 107| 107| 					"translateMessage": true
| 108| 108| 				},
| 109|    |-			wonderDuration)
|    | 109|+				wonderDuration)
| 110| 110| 		]
| 111| 111| 	};
| 112| 112| };
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/928/

lyv accepted this revision.Jan 5 2019, 9:09 PM

Looks good. Tested with all combinations of wonder and non-wonder upgrades.

This revision is now accepted and ready to land.Jan 5 2019, 9:09 PM
elexis added a subscriber: elexis.Jan 6 2019, 2:23 PM

Would it be possible to avoid creating a new timer instead of creating and deleting it? (Better avoid mistakes than knowingly doing and correcting them)
Maybe an early return.

bb added a comment.Jan 6 2019, 2:27 PM

well from a ownershipchange we can't see if it will be a rename, so when the new ent is brought up, we don't know it actually replaces another (one could change the entitylimits and allow two wonders, so an owners check wouldn't work), so the new timer will always be activated, which afterwards needs to be removed. Also from the destroy we don't know if it is a rename (one could very well create and destroy different wonders at the same turn), so we have to do it with the ent rename.

lyv added a comment.Jan 8 2019, 11:48 AM

(Traders also are not updated when a market renames if one wants to go all the way)

bb added a comment.Apr 6 2019, 11:58 PM

Calling the traders out of scope of this patch

This revision was automatically updated to reflect the committed changes.