Opened 8 years ago
Closed 8 years ago
#3769 closed defect (fixed)
[PATCH] Auras changing templates don't work
Reported by: | fatherbushido | Owned by: | elexis |
---|---|---|---|
Priority: | Must Have | Milestone: | Alpha 20 |
Component: | UI & Simulation | Keywords: | patch |
Cc: | Patch: |
Description
In aura templates
../0ad/binaries/data/mods/public/simulation/templates/units/athen_hero_pericles.xml ../0ad/binaries/data/mods/public/simulation/templates/units/ptol_hero_ptolemy_I.xml
code like
<Affects>Temple</Affects> <Modifications> <Cost.Resources.stone> <Add>-50</Add> </Cost.Resources.stone> </Modifications>
doesn't work.
But in Technology Modifications, like in ../0ad/binaries/data/mods/public/simulation/data/technologies/siege_cost_metal.json
"modifications": [{"value": "Cost/Resources/metal", "multiply": 0.80}], "affects": ["Siege"],
works.
Attachments (14)
Change History (29)
comment:2 by , 8 years ago
Milestone: | Alpha 21 → Backlog |
---|---|
Summary: | Modifications of Resources Cost seems broken in xml templates files → Auras changing templates don't work |
Thanks for reporting and investigating so far.
comment:4 by , 8 years ago
(The following was mentioned and discussed on #0ad-dev, I'm just summing up for the sake of anyone reading this in the future. Should they want to.)
With the above patch applied, the aura of Ptolomy I now works as expected, although not all the Ptolemaic mercenary units have the Mercenary
class. See #3770 for a patch for that.
In addition, the effect of Pericles' aura is now reflected in gui tooltips correctly, with the cost of a Temple showing as 250 stone. However, when a player places said building they are incorrectly charged 300 stone.
We're making progress!
See today's chat log for more details.
by , 8 years ago
Attachment: | patch_commands_js.diff added |
---|
comment:5 by , 8 years ago
I found the errors in command.js file.
Now there is still a problem : when we destroy foundations, the bad cost is given back.
We can modify fundation.js by using templates instead of cost.js but doing this we hide another problem.
Something seems broken in cost.js ( which comes from ApplyValueModificationsToEntity
in ValueModification.js
.... and finally from ApplyModifications
in AuraManager.js
).
comment:6 by , 8 years ago
Keywords: | patch added |
---|---|
Milestone: | Backlog → Alpha 20 |
Summary: | Auras changing templates don't work → [PATCH] Auras changing templates don't work |
by , 8 years ago
Attachment: | aurapatch.diff added |
---|
comment:7 by , 8 years ago
I modified some syntax as mimo suggested me.
FFollowing advices given by Sanderd17, it's better to not use Cost.js in Foundation.js because ApplyValueModificationsToEntity@ValueModification.js
doesn't work as expected with aura.
If ever someone want to use an hero aura with local aura which reduce ressource cost of structure, it will introduce some exploit (see :/irclogs).
by , 8 years ago
Attachment: | aurapatch.2.diff added |
---|
comment:9 by , 8 years ago
In Commands.js
you remove the line that declares template
- and then try to use said variable. Amusingly it works, because the variable had already been declared earlier. So that's fine. The variable cmpTemplateManager
is no longer used so you can safely remove that line too. And it is identical to cmpTemplateMgr
that's declared earlier anyway.
In Cost.js
, you split the for (let c of cache.keys())
into for (let [classeName,dataMap] of cache)
, permitting getting rid of various .get()
s, but the same has not been done to for (let key of dataMap.keys())
. Any reason for that?
On a general note, study the Coding_Conventions, if you haven't already, and pay attention to the section on formatting, especially with regard to brackets. Also, while not mentioned in the guide, we generally prefer let
over var
when declaring variables inside functions (it helps catch some of the re-declaration of variables such as mentioned above). (Unless leper or elexis tell you otherwise, you can leave pre-existing var
s as they are, just make sure you use let
with any new code you add)
As far as I can tell, the reason why auras (global or ranged) don't currently work as expected with foundation refunds: (most of it you seem to have worked out already)
- Player trains Pericles
- Aura comes into effect, effects template of Temple only
- Player places new Temple
- A new entity is created virtually by the game engine
- New entity is a structure so an instance of the Foundation component is init'd
- Foundation queries the cost of the Temple so it can store it in case of refunds.
- The cost is currently determined from the newly created entity, not from the template it is based on (your patch changes that)
- Having completed the initialization, the entity is placed into the game world.
- The sudden existence of a Temple is detected by the Aura component via
OnRangeUpdate
- Aura is applied to the new Temple entity, cost of that specific Temple entity updated
So:
- The Foundation component queries the cost when the entity is init'd
- The Aura component only applies the aura after it detects that a new entity of the appropriate type has been placed in world
by , 8 years ago
Attachment: | aurapatch.3.diff added |
---|
follow-up: 11 comment:10 by , 8 years ago
Thanks for all theses remarks, advices and for this sum up. I didn't want to introduce too many variables names but dataMap.get(key) is indeed use 3 times. I don't know what is better ?
comment:11 by , 8 years ago
Replying to fatherbushido:
Thanks for all theses remarks, advices and for this sum up. I didn't want to introduce too many variables names but dataMap.get(key) is indeed use 3 times. I don't know what is better ?
Looking at the XML, it seems dataMap -> modifications and dataMap.get(key) -> modification would fit. Also remove the unneeded braces.
by , 8 years ago
Attachment: | aurapatch.4.diff added |
---|
by , 8 years ago
Attachment: | aurapatch.5.diff added |
---|
by , 8 years ago
Attachment: | aurapatch.6.diff added |
---|
by , 8 years ago
Attachment: | aurapatch.7.diff added |
---|
by , 8 years ago
Attachment: | aurapatch.8.diff added |
---|
comment:13 by , 8 years ago
I add a new function in Cost.js, it's a cleaner way to do, following advices from Sanderd17. I edited some syntax following elexis remarks.
by , 8 years ago
Attachment: | aurapatch.9.diff added |
---|
comment:14 by , 8 years ago
As discussed in :irc, it's better to get Costs from templates instead of entity. So we edit the previous GetResourceCosts
.
The new function has an optional owner argument. It now gets costs from templates after modifications from aura and technologies with ApplyValueModificationsToTemplate
instead of getting costs from entity with ApplyValueModificationsToEntity
.
by , 8 years ago
Attachment: | aurapatch.10.diff added |
---|
by , 8 years ago
Attachment: | aurapatch.11.diff added |
---|
by , 8 years ago
Attachment: | aurapatch.12.diff added |
---|
It does not affect only
Cost.Resources
but alsoUnitMotion
as.does not work in aura xml template. But
works in json technology file.