HomeWildfire Games

Rewrite Structure Tree and Template Viewer to use OOP principles
AuditedrP23808

Description

Rewrite Structure Tree and Template Viewer to use OOP principles

Commenters: elexis, Stan, Angen
Refs.: #5387
Differential Revision: https://code.wildfiregames.com/D2734

Event Timeline

Stan added a subscriber: Stan.Jul 11 2020, 6:02 PM

I think we might have overlooked something https://wildfiregames.com/forum/index.php?/topic/28491-archery-tradition-for-all-factions/&tab=comments#comment-400664
I looked at the code, but couldn't find where the issue lies

Should be remedied by rP23816

Nescio added a subscriber: Nescio.Jul 30 2020, 5:44 PM

@s0600204, there is something wrong with the values displayed in the structure tree when launched from the pre-game main menu. It worked correctly before, therefore my guess is the problem was introduced here.
For example, palisades have 200 to 1000 health. athen, mace, spart structures have +10% health as a civ bonus, so for them it is 220 to 1100 health, but not for the others. When opened from a game session, the structure tree displays the correct values, e.g. 200 to 1000 for pers. However, when launching the structure from the pre-game main menu, it displays 220 to 1100 health for all civs, which is wrong.

@s0600204 If you have some time, could #5848 be a result of this commit?

[...] could #5848 be a result of this commit?

It is not. This commit just made the bug more obvious.

Oh okay, cool. Thanks for looking into it :)

Silier raised a concern with this commit.Dec 29 2020, 3:43 PM
Silier added subscribers: wowgetoffyourcellphone, Silier.

This is applying autoresearched techs acrosss all civs.
Not case in a23b, checked with carth walls autoresearched tech.
Noticed by @wowgetoffyourcellphone on forum https://wildfiregames.com/forum/topic/28516-bug-reports/?do=findComment&comment=413171

This commit now has outstanding concerns.Dec 29 2020, 3:43 PM
Silier resigned from this commit.Dec 29 2020, 3:48 PM

ups, sorry :) I missed something

This commit no longer requires audit.Dec 29 2020, 3:48 PM

Hi @s0600204 ,
when unit with gaia civ has required phase, it is not in correct row.
Does not happen in a23b.

Silier raised a concern with this commit.Jan 12 2021, 9:10 PM
Silier added inline comments.
/ps/trunk/binaries/data/mods/public/gui/reference/common/TemplateParser.js
47

I would dissagree.
One can train gaia animals in corrals and still have required technology on phase

This commit now has outstanding concerns.Jan 12 2021, 9:10 PM
Silier added inline comments.Jan 12 2021, 9:58 PM
/ps/trunk/binaries/data/mods/public/gui/reference/common/TemplateLoader.js
123

getBaseTemplateName is problematic,
if you look into women trained from house, they have higher production cost, by calling this, structure tree shows women trained from cc

Silier added inline comments.Jan 13 2021, 8:59 AM
/ps/trunk/binaries/data/mods/public/gui/reference/common/TemplateLister.js
115

functionally maybe, but not neccesary with the same cost

s0600204 requested verification of this commit.Apr 14 2021, 10:09 PM
s0600204 added inline comments.
/ps/trunk/binaries/data/mods/public/gui/reference/common/TemplateParser.js
47

Sorry: I meant gaia-owned objects.

This commit now requires verification by auditors.Apr 14 2021, 10:09 PM
Silier accepted this commit.Apr 14 2021, 11:42 PM
All concerns with this commit have now been addressed.Apr 14 2021, 11:42 PM