endless-sky / endless-sky

Space exploration, trading, and combat game.
https://endless-sky.github.io/
GNU General Public License v3.0
5.82k stars 1.03k forks source link

The `require "Jump Drive"` vs plugins problem and a possible solution approach? #9720

Open SomeTroglodyte opened 9 months ago

SomeTroglodyte commented 9 months ago

Problem Description

Several missions test for the "Jump Drive" outfit when story-wise they should be testing for jump capability independent of source (Only relevant with plugins at the moment methinks).

Now, the "flagship attribute: jump drive" condition works perfectly fine, but some missions, e.g. "lost in coalition" or "Coalition: Expeditions Past 1", do not use to conditions but on.. require.

Related Issue Links

9308

I'm sure there's others...

Desired Solution

Why not make the "require" node accept conditions?

Testbed data: ``` mission "lost in coalition" landing invisible source government "Coalition" "Heliarch" to offer has "license: Coalition" on offer require not "flagship attribute: jump drive" event "lost in coalition" 1 fail event "lost in coalition" ... mission "Coalition: Expeditions Past 1" minor name "Scouting Permit" description "Transport Sedlitaris, a Heliarch Tribune, to the , where she will petition the Heliarch council to allow her to go on a scouting expedition out of the Coalition. Payment is ." passengers 1 to offer "coalition jobs" >= 40 random < 50 source government "Coalition" destination "Ring of Friendship" on offer require has "flagship attribute: jump drive" ... ```
Patch: ```patch Index: source/MissionAction.cpp IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/source/MissionAction.cpp b/source/MissionAction.cpp --- a/source/MissionAction.cpp (revision 460e3c426dd39767526992aae387083bbb947df6) +++ b/source/MissionAction.cpp (date 1706128598179) @@ -82,17 +82,18 @@ { const string &key = child.Token(0); bool hasValue = (child.Size() >= 2); + bool hasChildren = child.HasChildren(); if(key == "dialog") { if(hasValue && child.Token(1) == "phrase") { - if(!child.HasChildren() && child.Size() == 3) + if(!hasChildren && child.Size() == 3) dialogPhrase = ExclusiveItem(GameData::Phrases().Get(child.Token(2))); else child.PrintTrace("Skipping unsupported dialog phrase syntax:"); } - else if(!hasValue && child.HasChildren() && (*child.begin()).Token(0) == "phrase") + else if(!hasValue && hasChildren && (*child.begin()).Token(0) == "phrase") { const DataNode &firstGrand = (*child.begin()); if(firstGrand.Size() == 1 && firstGrand.HasChildren()) @@ -115,6 +116,10 @@ else child.PrintTrace("Error: Skipping invalid \"require\" count:"); } + else if(key == "require" && !hasValue && hasChildren) + { + requiredConditions.Load(child); + } // The legacy syntax "outfit 0" means "the player must have this outfit installed." else if(key == "outfit" && child.Size() >= 3 && child.Token(2) == "0") { @@ -174,6 +179,15 @@ conversation->Save(out); for(const auto &it : requiredOutfits) out.Write("require", it.first->TrueName(), it.second); + if(!requiredConditions.IsEmpty()) + { + out.Write("require"); + out.BeginChild(); + { + requiredConditions.Save(out); + } + out.EndChild(); + } action.Save(out); } @@ -288,6 +302,9 @@ } } + if(!requiredConditions.Test(player.Conditions())) + return false; + // An `on enter` MissionAction may have defined a LocationFilter that // specifies the systems in which it can occur. if(!systemFilter.IsEmpty() && !systemFilter.Matches(player.GetSystem())) @@ -363,6 +380,7 @@ result.systemFilter = systemFilter.SetOrigin(origin); result.requiredOutfits = requiredOutfits; + result.requiredConditions = requiredConditions; string previousPayment = subs[""]; string previousFine = subs[""]; Index: source/MissionAction.h IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/source/MissionAction.h b/source/MissionAction.h --- a/source/MissionAction.h (revision 460e3c426dd39767526992aae387083bbb947df6) +++ b/source/MissionAction.h (date 1706127136197) @@ -88,6 +88,9 @@ // Outfits that are required to be owned (or not) for this action to be performable. std::map requiredOutfits; + // Conditions that must be fulfilled for this action to be performable. + ConditionSet requiredConditions; + // Tasks this mission action performs, such as modifying accounts, inventory, or conditions. GameAction action; }; ```

This tests just fine for me.

Alternative Approaches

Put the conditions in a to node - but I do not know enough about the project to guess why the on..require mechanism is used instead of, say, to offer.

Additional Context

I can compile and run, but not debug so far - so my testing might be a little on the short side.

ziproot commented 8 months ago

I think require was the older way of doing it and not everything was changed

Hurleveur commented 8 months ago

For the missions that want to take a Jump Drive from you we still need to keep that old form, but for other missions that check for being stuck we can indeed change it to the new method.

SomeTroglodyte commented 8 months ago

want to take a Jump Drive from you

Exactly. My patched Emerald Sword mission got a little complicated to ensure it's only OK when you can still jump after losing one "Jump Drive" outfit. One needs to be the exact outfit, the other can have whatever source.