#1358 closed enhancement (fixed)
[PATCH] Extend technology manager for template data
Reported by: | historic_bruno | Owned by: | ben |
---|---|---|---|
Priority: | Must Have | Milestone: | Alpha 11 |
Component: | UI & Simulation | Keywords: | patch |
Cc: | Jonathan Waller | Patch: |
Description (last modified by )
The new tech system currently supports applying stat modifications to existing entities. But we often need the stats prior to creating the entity, for information/UI or e.g. using costs to calculate if we have sufficient resources to create the entity.
See below for discussion.
Attachments (1)
Change History (13)
comment:1 by , 12 years ago
comment:2 by , 12 years ago
Milestone: | Alpha 10 → Alpha 11 |
---|
Had some discussion about this in IRC today. To summarize what is by no means a consensus:
- There are three separate areas that need access to tech-related data: simulation, GUI, and AIs:
- simulation just needs to use modified entity stats after techs are applied, the current design allows for this
- the GUI needs to reflect which techs are researchable and when things are unlocked, the current design achieves this
- the GUI also needs to know updated stats for trainable and buildable templates, which is currently lacking
- the AI also needs to know about techs, unlocking, and modified stats, but unlike the GUI, it doesn't have GUIInterface (only a simulation state proxy updated each turn)
- We shouldn't arbitrarily limit which stats GUI and AI have access to, that complicates modding
Proposed design:
- Factor out the actual tech modification code from TechnologyManager into a new global script accessible by simulation, GUI, and AI
- The GUI and AI get a list of all entity and tech template data at the start of the game
- Each turn they get an updated list of researched techs for each(?) player. The AI might also have use for
TechnologyModification
and/orTechnologyResearched
events. - In the global tech script will be some logic that takes as input:
- all tech template data
- a list of techs researched by each(?) player
- a player id (who would own the entity)
- an entity template (of the entity to be affected)
- a property name (of the modification type to look for)
- It returns the possibly modified value of the property given those inputs
- The function call might look like:
var healHP = GetModifiedAttribute(g_Templates, g_TechData, techsForCurrentTurn, playerID, templateName, "Heal/HP");
- The simulation may handle it slightly differently
That design helps to minimize the amount of data computed each turn and tech modifications are only accessed on demand, instead of being "pushed" into the GUI/AIs. It also naturally preserves the original template data, so we don't need separate functions to access base vs. bonused stats.
I think we can make due without this until Alpha 11. More discussion is warranted.
comment:3 by , 12 years ago
Description: | modified (diff) |
---|
comment:4 by , 12 years ago
Priority: | Should Have → Must Have |
---|
comment:5 by , 12 years ago
Description: | modified (diff) |
---|
follow-up: 7 comment:6 by , 12 years ago
Thinking about this some more, assuming we use consistent tech modification names based on the entity templates, could we not support techs for all possible template values at once? Imagine passing in some template data to the proposed global tech function:
"ResourceGatherer": { "MaxDistance": 2.0, "BaseSpeed": 1.0, "Rates": { "food.fish": 1, "metal.ore": 3, "stone.rock": 3, "wood.tree": 2 }, "Capacities": { "food": 10, "metal": 10, "stone": 10, "wood": 10 } }
It could iterate all properties of the template object, construct the tech modification name for each (e.g. "ResourceGather/MaxDistance"
or "ResourceGatherer/Capacities/food"
) and retrieve the (cached) modified value. A new object would be returned and accessed as if there were no techs. That seems like a very clean way of accessing the modified data - basically one call per component and the component doesn't need to know what techs exist.
Mostly I'm thinking of the vision of 0 A.D. as a mod-friendly game, but it's not mod-friendly if we have a limited subset of stats that can be modified by techs (currently every tech is represented by a separate function call to the tech manager component).
comment:7 by , 12 years ago
Replying to historic_bruno:
Thinking about this some more, assuming we use consistent tech modification names based on the entity templates, could we not support techs for all possible template values at once?
Unfortunately, as nice as that would be, it won't work with our current entity template system. The reason is the template data are all strings by necessity, it's impossible to globally know what type is desired, only the code that accesses them knows the type.
Philip raised another good point: even if we solved the type issue, we couldn't guarantee that the simulation will handle an arbitrary changing stat correctly -- it may require one or more messages being handled and other special handling (LOS is a good example), we need to extend the simulation on a case-by-case basis. See the IRC discussion on 2012-06-14 starting at 23:43, continued on 2012-06-15.
Otherwise I'm looking into this ticket, here's what I'm thinking currently for the global function:
function GetTechModifiedProperty( techModifications, entityTemplate, propertyName, propertyValue )
techModifications
: comes from the tech manager which stores a mapping of propertyName -> modificationsArray for the player's researched techs, it needs to be sent to UI/AI each turn. IMO it's simpler to reuse that data because it already encapsulates both researched techs and the parsed modifications data.entityTemplate
: raw template for the given entity, currently only used for itsIdentity.Classes
element - maybe we can just pass in a classes array?propertyName
: the property we want to modify, e.g."Heal/HP"
. It's a key intotechModifications
.propertyValue
: current value of the property being modified, we have to do this because we can't infer the type from the entityTemplate (see above).
The logic is essentially what exists now in CmpTechManager.ApplyModificationsWorker()
but relative to a template, not an entity.
by , 12 years ago
Attachment: | 1358-git.patch added |
---|
comment:8 by , 12 years ago
Attached git patch that does what I described in my last comment. It seems to work well enough for UI and simulation. I also modified the AI slightly to begin supporting techs. I'm less sure about the AI changes and it's harder to test since AIs don't have any tech handling yet. It could be seen as an incomplete start to adding full tech support to AIs.
Overall the patch is much cleaner than I had anticipated, I think that's a good sign :)
Notes:
- I refactored
ScriptInterface::ReplaceNondeterministicFunctions()
into separate functions, to make it easier to load arbitrary global scripts. - I decided to use existing functions in
GuiInterface
which offers a few nice benefits: AI and UI get the same modifications data (viaGetSimulationState()
), andGetTemplateData()
is already cached by the UI and flushed each turn -- so why not apply the techs there? Keeping the tech modifications separate from UI logic is nicer for modding, too. - The AI needs some additional handling for
Entity
andEntityTemplate
objects, namely tech research messages need to be passed through the AI and then used to selectively update the_techModifications
property of theEntityTemplate
- as we do for other properties. I thought this could be left as a TODO since #1410 is unfixed. - I don't like the duplication of almost identical function calls in 3 places:
GuiInterface
, AI (entity.js
), and other simulation components (throughTechnologyManager
), but it seems a necessary evil given the above concerns. At least they are confined to simulation and common-api-v2, which allows new GUIs and AIs to be designed without hard-coding possible technologies.
comment:9 by , 12 years ago
Keywords: | patch review added |
---|---|
Summary: | Extend technology manager for template data → [PATCH] Extend technology manager for template data |
comment:10 by , 12 years ago
Cc: | added |
---|
comment:12 by , 8 years ago
Keywords: | review removed |
---|
Agreed. The current system, while working, is limited. By default, accessing something like
this.entity.template.max_health
should already be stat modified.I think the following names would make the most sense:
The other thing too... the tech engine doesn't support entities with an ID of -1. So when you contruct a farm, the entity id is -1 for some reason. As a result, when the farm finishes constructing, you can't add techs to it or it'll barf, so any max health increases won't work as intended :-( I found this out trying to increase farms max produce.