Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#4255 closed defect (fixed)

[PATCH] When upgrading the costs are substracted then readded.

Reported by: fatherbushido Owned by: elexis
Priority: Must Have Milestone: Alpha 21
Component: UI & Simulation Keywords: patch
Cc: Patch:


Since r18470, the upgrades are free. It is currently used for walls -> gate, wooden tower -> defense tower. It comes from that the upgrade is cancelled when the ownership of the upgraded units changes (so when the old entity is deleted) as it was destroyed.

We can for example change the way the IsUpgrading function works or adding a boolean in the Upgrade component.

Attachments (1)

upgrade.diff (4.5 KB ) - added by fatherbushido 6 years ago.

Download all attachments as: .zip

Change History (7)

by fatherbushido, 6 years ago

Attachment: upgrade.diff added

comment:1 by elexis, 6 years ago

Keywords: review removed
  • Not calling CancelUpgrade on ownershipchange seems wrong since it resets the timer which is a system entity. If the upgrade finishes, will that timer component (Timer.js) run code for that timer forever (without any purpose, potentially causing bugs)? You could instead pass a refund boolean to that function.
  • Should the player receive a total refund of the upgrade cost on capture or destruction? Is it done the same way for researching techs / production queue on ownership change?
  • Should the code set this.completed = true even before the CancelTimer call in the UpgradeProgress function (which is called periodically by just that timer)? I can't construct any case that would make it an issue though.

comment:2 by fatherbushido, 6 years ago

  • in such a case, it is done in UpgradeProgress
  • yes, it's designed like that
  • don't know

comment:3 by elexis, 6 years ago

But is UpgradeProgress actually called after the entity is destroyed?

comment:4 by elexis, 6 years ago

Priority: Release BlockerMust Have

Nevermind, there can't really be a concurrency issue since the timer is called depending on the simulation time, and that time is only forwarded once the code for the current turn had been processed, so no race condition unless I'm overseeing something.

Since CancelTimer is called in UpgradeProgress when the update completes, that timer won't be an issue.

comment:5 by elexis, 6 years ago

Owner: set to elexis
Resolution: fixed
Status: newclosed

In 18791:

Don't refund the upgrade costs when the upgrade completes. Patch by fatherbushido, fixes #4255, refs #2706.

comment:6 by fatherbushido, 6 years ago

In 18796:

Fixes a counter mistake when an upgrade is complete. Reviewed by elexis. Refs #4255, #2706.

Note: See TracTickets for help on using tickets.