Opened 7 years ago
Last modified 5 years ago
#4628 new defect
Prototype inheritance for Trigger scripts
Reported by: | elexis | Owned by: | |
---|---|---|---|
Priority: | Should Have | Milestone: | Backlog |
Component: | Simulation | Keywords: | |
Cc: | Krinkle | Patch: |
Description
All map and mod based scripting (that isn't part of 0 A.D. or pyrogenesis mainline features) like
- victory condition scripts (like Conquest, Regicide, CaptureTheRelic, WonderVictory,...)
- map trigger scripts (like on survival of the fittest, danubius or extinct volcano)
- the tutorial from Phab:D194
currently extend the Trigger
system component.
A big issue with this is that by adding more and more of these scripts, the possibility to overwrite one variable of another file extending the same prototype becomes increasingly probable.
For example we keep track of heroes in both Regicide.js
(cmpTrigger.regicideHeroes
), danubius_triggers.js
(cmpTrigger.heroes
).
To avoid overwrite, we use unpleasent naming (we don't really need the regicide
in the variable name if the file is already about regicide only).
Instead we should use actual prototype inheritance for this system component.
At https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/call we can find some information on how we can extend the existing prototype:
Regicide.prototype = Object.create(Trigger.prototype);
and to call a method of the parent:
Trigger.prototype.Init.call(this);
In order to register these trigger scripts as actual new system component, they need to register the interface in new files in a new directory.
A second positive effect of this refactoring is that we can remove the ugly proxies from Trigger.js
, for example
Trigger.prototype.OnGlobalOwnershipChanged = function(msg) { this.CallEvent("OwnershipChanged", msg); // data is {"entity": ent, "from": playerId, "to": playerId} };
and subscribe to all messages without hardcoding a small subset of message names and adding a duplicate proxy for all of these:
Regicide.prototype.OnGlobalOwnershipChanged = function(data) { ... };
Initialization order is important.
If the prototype and the inherited prototype reside in the same directory, but the file with the prototype inheriting the other prototype is loaded first, it will complain about that not being existant.
This will be relevant in case we revert the decision from r15427 to use the maps/ directory for victory conditions and move them to a new victory condition directory, probably in simulation/
(I don't see anyone implementing variants of victory conditions for each map. These gametypes are supposed to work with all maps. Maybe we can add restrictions to skirmish and random maps, but that still doesn't require us to make this a map/
specific thing).
Change History (4)
comment:1 by , 7 years ago
Patch: | → Phab:D626 |
---|
comment:2 by , 5 years ago
Component: | UI & Simulation → Simulation |
---|---|
Patch: | Phab:D626 |
comment:3 by , 5 years ago
Cc: | added |
---|
comment:4 by , 5 years ago
As mentioned in the revision proposal, the added complexity of using prototype inheritance might not be necessary to address the problem of property name conflicts, since the affected trigger component extensions (regicide, capture the relic...), might become individual components without using inheritance. On the other hand the possible Trigger property naming conflict problem remains as long as it is recommended to extend the Trigger component (or components in general).
Backlogging.