Opened 3 years ago

Closed 3 months ago

#4334 closed defect (fixed)

[PATCH] Make Promotion use the common helper in Transform.js and update Transform.js

Reported by: wraitii Owned by: wraitii
Priority: Should Have Milestone: Alpha 24
Component: Simulation Keywords: patch
Cc: Patch: Phab:D2026

Description (last modified by fatherbushido)

Promotion currently doesn't use the ChangeEntityTemplate? function defined in Transform.js, which is bad, and it should.

In parallel, that function does not correctly copy enough things. The attached patch fixes that.

Attachments (3)

TransformFix.patch (7.8 KB) - added by wraitii 3 years ago.
TransformFix2.patch (9.3 KB) - added by wraitii 3 years ago.
promotion.jpg (235.4 KB) - added by fatherbushido 3 years ago.

Download all attachments as: .zip

Change History (13)

Changed 3 years ago by wraitii

Attachment: TransformFix.patch added

comment:1 Changed 3 years ago by fatherbushido

I did a quick test (not a review).

  • I have no opinion on what should be copied or not: what is choosen in the patch seems good.
  • I have strange issue with the patch:
    • when a unit is moving, patrolling or gathering and is promoted, it does its cheering animation with sliding (funny to see) and then stop the order. It was not like that before (doing the cheering animation staying idle, then continuing the previous order).
    • when a unit is visibly garrisoned and is promoted, it does the cheering animation but the selection ring appears and does some circular moves. It's not really annoying but it was not like that before.
Last edited 3 years ago by fatherbushido (previous) (diff)

Changed 3 years ago by wraitii

Attachment: TransformFix2.patch added

comment:2 Changed 3 years ago by wraitii

Fixed point 1 by making the unit cheer after the other orders are completed. This isn't perfect, but given the change it's impossible to do better.

Fixed point 2 by preventing promotion of garrisoned entities (see comment in Transform.JS).

comment:3 Changed 3 years ago by fatherbushido

I did a test of TransformFix2.patch​ I don't know if preventing promotion of garrisoned entities is wanted. Else test result:

  • Garrison a macedonian spearman champ. Search the silver shield tech. Ungarrison the unit. It is not promoted and has an infinite xp bar (see screenshot for fun).
  • (Visible)Garrison a range unit on a wall. When it get enough xp to promote, the xp bar is just reseted and start filling again. Ungarisonning it of the wall let him that new xp and don't promote it.

http://trac.wildfiregames.com/raw-attachment/ticket/4334/promotion.jpg

Last edited 3 years ago by fatherbushido (previous) (diff)

Changed 3 years ago by fatherbushido

Attachment: promotion.jpg added

comment:4 Changed 3 years ago by fatherbushido

Keywords: review removed

comment:5 Changed 3 years ago by elexis

Milestone: Alpha 22Backlog

Backlogging due to lack of progress

comment:6 Changed 2 years ago by fatherbushido

Description: modified (diff)

comment:7 Changed 9 months ago by Imarok

Component: UI & SimulationSimulation

Move tickets to Simulation as UI & Simulation got some sub components.

comment:8 Changed 5 months ago by elexis

comment:9 Changed 3 months ago by wraitii

Milestone: BacklogAlpha 24
Patch: Phab:D2026

comment:10 Changed 3 months ago by wraitii

Owner: set to wraitii
Resolution: fixed
Status: newclosed

In 22753:

Make Promotion.js use the common Transform helper, add resource gatherer and promotion to said helper.

Fixes #4334

Differential Revision: https://code.wildfiregames.com/D2026

Note: See TracTickets for help on using tickets.