Trigger Attachments. If you maps has them, please read this.
I am thinking of re-writing how Triggers work.
Depending on how you coded your map xml, this might affect you or not.
One of my goals, is to NOT affect your map. I don't want anyone to have to re-write their map because of my changes.
However, I only know how I coded my maps, I don't know what kinds of crazy things you guys did.
HOW I CODED MY XML TRIGGERS: Each of the triggers in my maps does only 1 thing.
If I need to do 2 things, I made 2 different triggers.
So for example, I may want to do 2 things: give a player a tech, and switch their production frontier.
I want this to happen on this condition: I have conquered Moscow for the first time.
What I do in my maps: ConditionAttachment_Germans_control_Moscow -> has the condition of direct ownership of Moscow by Germany
TriggerAttachment_Germans_ReceiveTechX -> has the condition above, has 'uses=1', and has 'tech' equal to 'tech X'
TriggerAttachment_Germans_ProductionFrontier_changedTo_Blah -> has the condition above, has 'uses=1', and has 'frontier' equal to 'German_Blah_production'
Therefore, when the condition is true, I will have two separate triggers firing. This is the BEST way of doing things right now.
However, the engine currently allows this following method. I consider this method to be BAD PRACTICE. See below:
ConditionAttachment_Germans_control_Moscow -> has the condition of direct ownership of Moscow by Germany
TriggerAttachment_Germans_ReceiveTechX_and_ProductionFrontier_changedTo_Blah -> has the condition above, has 'uses=1', and has 'tech' equal to 'tech X', and has 'frontier' equal to 'German_Blah_production'
So, my question is, DOES ANYONE USE THIS 'BAD' WAY OF CODING THEIR TRIGGERS?
I want to change the way triggers work, and it will NOT change the "GOOD" way, but it will change how the current "BAD" way is implemented when you actually play.
Here is a brief overview of why, which you don't need to read if you don't want to:
One thing I want to point out, that makes any refactoring of the Trigger attachment difficult is this:
The current way of "activating" a trigger, does NOT activate the FULL trigger.
Instead, it activates only a part of the trigger.
For example, take this:
in Trigger Attachment: public static void triggerTechChange(PlayerID player, IDelegateBridge aBridge, final String beforeOrAfter, final String stepName)
This is called from exactly two places: the TechActivationDelegate, and the BaseDelegate.
In the TechActivationDelegate, we are activating this trigger IF this trigger has no "when" variable set.
In BaseDelegate, we are activating this trigger IF the trigger has a "when" set.
Now, inside this, we go and get the triggers that HAVE tech changes: Set<TriggerAttachment> trigs = getTriggers(player, data, techMatch(beforeOrAfter, stepName));
This is done by using this match:
public boolean match(TriggerAttachment t)
return t.getUses() != 0 && whenOrDefaultMatch(beforeOrAfter, stepName).match(t) && !t.getTech().isEmpty();
Which means, if the Trigger has uses still (either uses is >0 or uses is -1 which means infinite), and it has a "when" either null or matching the target's "when", and the "t.getTech()" is not empty.
Basically, the "t.getTech()" is what determines that this trigger is going to fire or not. Any trigger in the entire map xml that has t.getTech() set to something, will come up for firing. Any trigger that doesn't have t.getTech() set, will be ignored.
Now, we take this list of triggers that has t.getTech() set, and see if any of them have their conditions match:
for (TriggerAttachment t : trigs)
boolean met = isMet(t, data);
Then, if they are met, we actually perform JUST the tech-activation PART of the trigger. We do not perform all the parts of the trigger, just the tech activation:
for (PlayerID aPlayer : t.getPlayers())
for (TechAdvance ta : t.getTech())
|| !TechAdvance.getTechAdvances(data, aPlayer).contains(ta))
aBridge.getHistoryWriter().startEvent(MyFormatter.attachmentNameToText(t.getName()) + ": " + aPlayer.getName() + " activates " + ta);
TechTracker.addAdvance(aPlayer, aBridge, ta);
In other words, if there is a trigger that does 2 things: gives the player a new technology, and switches their production frontier to something else, then that trigger will fire in 2 different places, with each part firing once.
So, lets say this trigger has the following things:
condition - "we do not have technology X"
trigger tech activation - "give technology X to player"
trigger production frontier switch - "switch production frontier to 'blah'"
Now, lets say that this trigger is called in the following order:
(we start without tech X)
What will happen is as follows:
triggerTechChange will find all triggers with a tech change as part of the trigger. This list will include our triggerattachment, TriggerAttachment_TechX_and_FrontierBlah.
triggerTechChange will look at the conditions of TriggerAttachment_TechX_and_FrontierBlah
TriggerAttachment_TechX_and_FrontierBlah's condition is true, because we do not have "tech X".
triggerTechChange will fire the tech change of TriggerAttachment_TechX_and_FrontierBlah, giving the player "tech X".
triggerTechChange returns, and is done
triggerProductionChange starts, and finds all triggers with a production change. This list will include our triggerattachment, TriggerAttachment_TechX_and_FrontierBlah.
triggerProductionChange will look at the conditions of TriggerAttachment_TechX_and_FrontierBlah
TriggerAttachment_TechX_and_FrontierBlah's condition is FALSE, because we now have "tech X".
triggerProductionChange will return, having done nothing.
At this point, you might be asking yourself: "this sounds like a stupid way of doing things"
If so, I'm tempted to agree with you.
So, before we "fix" or "refactor" TriggerAttachment, we need to agree on what SHOULD be done.
Should we maintain the above behavior, since it is how the engine currently works, and some maps might be affected.
Or should we have the engine only look to fire Triggers once, and then when it tests the conditions, it should fire all the triggers who's conditions are true, including all the different parts of the trigger?
Whatever we do decide, we need to make sure we can still "fire" the individual parts of the trigger separately. In other words, we need to be able to fire the "techChange" separately from the "productionChange". The reason being that the "productionChange" generally occurs at the beginning of the turn, while the "techChange" occurs at the end of the turn. If they both fired at the same time, this would change a lot of maps and I can not allow that (for example, it would give a player the tech at the start of their turn, which would impact that turn).