Page MenuHomeWildfire Games

possible hotfix for wrong tech tooltips and structure tree
AcceptedPublic

Authored by marder on Wed, Aug 3, 10:10 PM.

Details

Reviewers
Stan
Freagarach
Trac Tickets
#6587
Summary

https://trac.wildfiregames.com/changeset/26015
removed some default values, namely the TechCostMultiplier

So why not just add them back in?
From my quick test everything seem to work properly now.

The macedonian storehouse tooltip displays now no time at all.
The mauryan temple has only half the tech resource cost in game and in the structure tree.
Saving a game still works.

Test Plan

Double check if everything works properly.

I kind of suspect I may have overlooked some obvious problem as the fix seems too simple.

Event Timeline

marder created this revision.Wed, Aug 3, 10:10 PM

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

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

Build is unstable, some tests have failed - The Moirai have given mortals hearts that can endure.

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

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

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

marder requested review of this revision.Wed, Aug 3, 10:26 PM
marder edited the summary of this revision. (Show Details)Wed, Aug 3, 10:29 PM
marder edited the test plan for this revision. (Show Details)
marder updated this revision to Diff 20708.EditedThu, Aug 4, 1:01 AM

checkrefs was complaining again:

  File "checkrefs.py", line 288, in add_entities
    techString = cmp_researcher.find('Technologies').text 
AttributeError: 'NoneType' object has no attribute 'text'

which I believe is a false error as templates are allowed to have a TechCostMultiplier

therefore changing checkrefs so that it doesn't complain anymore and the CI will hopefully be happy

Vulcan added a comment.Thu, Aug 4, 1:06 AM

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

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

marder added a comment.Thu, Aug 4, 1:06 AM

Note that: python checkrefs.py -tax still gives me locally the following error:

File "0ad\source\tools\entity\../xmlvalidator\validate_grammar.py", line 8, in <module>
    import lxml.etree
ModuleNotFoundError: No module named 'lxml'

but that seems like it has nothing to do with this patch

Vulcan added a comment.Thu, Aug 4, 1:17 AM

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

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

marder added inline comments.Thu, Aug 4, 1:26 AM
binaries/data/mods/public/simulation/templates/template_structure.xml
96

wrong indent

I thought of this just yesterday. ^^' I guess I'm too busy to think clearly.
Thanks for the patch!

Notice that if you right-click the icon in the PQ of a mace storehouse it will show the wrong value. But this patch is an improvement nonetheless.

P277 complains as well. (There should be more template files listed, but after the first entry it stops.)

Production Queue Validator
❯ python3 Production.py -v -r .
INFO - Launching 0.A.D with the following command binaries/system/pyrogenesis -mod="mod" -mod="public" --rl-interface="127.0.0.1:9090" --autostart-nonvisual --autostart="skirmishes/acropolis_bay_2p"
INFO - Calling http://127.0.0.1:9090/evaluate to get data
INFO - Killing 0 A.D.
INFO - Looking for templates with missing production queue.
ERROR - structures/generic_field has no production queue.

@s0600204 is the one who wrote the structure tree and recommends to do the fix via globalscripts.

Freagarach accepted this revision.Thu, Aug 4, 8:10 AM

Fix the indent, add it only to structures doing actual research (fixes the checkrefs) and ship it, I'd say. A globalscript fix would be much more correct, but also way more work with added chances of breakage.

This revision is now accepted and ready to land.Thu, Aug 4, 8:10 AM

P277 complains as well. (There should be more template files listed, but after the first entry it stops.)
@s0600204 is the one who wrote the structure tree and recommends to do the fix via globalscripts.

Well that was what I tried at the beginning, but I got an annoying error that ApplyValueModificationsToTemplate() is not a function and had no time to investigate further.

A globalscript fix would be much more correct, but also way more work with added chances of breakage.

agreed.

Fix the indent, add it only to structures doing actual research (fixes the checkrefs) and ship it, I'd say.

Unfortunately I won't have time until next week, so if you want to, feel free to take over.
Isn't the checkrefs change still correct tho? Why should we check if it's none/ or should we expect all tech to have a text element?

Stan added a comment.Thu, Aug 4, 11:09 AM

Wait why would one have Researcher with no techs? Only tech removal?

For -tax to work pip install libxml

In D4750#202208, @Stan wrote:

Wait why would one have Researcher with no techs? Only tech removal?

The schema for the Researcher component specifies that the <Technologies/> tag is optional. If omitted, then cmp_researcher.find('Technologies') will return None because there simply isn't an element by that name on the cmp_researcher xml-dom node in such a case.

This is made more obvious by this patch, as it gives all structures a Researcher component[^1] but no <Technologies/>to go with it.

[^1] - But not a ProductionQueue component, which is presumably what the P277 script is complaining about.

A globalscript fix would be [...] way more work with added chances of breakage.

Indeed it was (and is): P282

I haven't exhaustively tested it; by all means leave it until after A26 gets out the door.

marder added a comment.Mon, Aug 8, 7:31 PM

A globalscript fix would be [...] way more work with added chances of breakage.

Indeed it was (and is): P282

I haven't exhaustively tested it; by all means leave it until after A26 gets out the door.

I tested it and it looks good/ I couldn't find and error.
Struct tree looks good, in game looks good, no errors, checkrefs ok.