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 elexis, 7 years ago

Patch: Phab:D626

comment:2 by Itms, 5 years ago

Component: UI & SimulationSimulation
Patch: Phab:D626

Backlogging.

comment:3 by Krinkle, 5 years ago

Cc: Krinkle added

comment:4 by elexis, 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).

Note: See TracTickets for help on using tickets.