henricj/dunelegacy

Spontaneous "House 'Fremen' has been defeated." is printed when a sand worm dies (or eats three units)

juj opened this issue · 4 comments

juj commented

Playing Ordos Level 5, I spotted an interesting log message

"House 'Harkonnen' has been defeated."

while the mission was me (Ordos) against Atreides, and there were no Harkonnen reinforcements or anything like that available to Atreides.

image

juj commented

Hmm, oddly, level 6 mentat intro refers to Sardaukar having attacked the player in level 5:

image

However I am pretty sure I never saw any Sardaukar troops during level 5 playthrough. That makes me wonder if there were supposed to be some Harkonnen/Sardaukar troops during that level.

With regard to the log message, it is coming from here:

fmt::sprintf(_("House '%s' has been defeated."), getHouseNameByNumber(getHouseID())));

I'm not sure why the house would be Harkonnen at that point. If you can repo, try saving the game shortly before the game is lost.

As for the second, are there any warnings about Sardukar in the log file? I have some idea that the GCC and/or Clang builds warn about one of the menu classes having an unused field named "mission," or it could be another victim of my campaign to get rid of hidden/shadowed names.

juj commented

After the PR #31, the spontaneous "House 'Harkonnen' has been defeated." now turned into a "House 'Fremen' has been defeated."

Observed this now in Harkonnen level 7.

This is related to the Sand Worms. Breaking in debugger, I see House::lose() begin called on HOUSE_FREMEN.

image

The call stack is in some kind of delayed delete/cleanup processing.

Backing up, if I put a breakpoint to Sandworm's die logic:

image

It is caught, and right on next frame the House::lose() on Fremen is triggered.

The function

void UnitBase::cleanup(const GameContext& context, HumanPlayer* humanPlayer) {

calls

game.getHouse(originalHouseID_)->decrementUnits(itemID_);

which calls House::lose() if the house is now without units:

    if (!isAlive())
        lose();

So in summary, what seems to happen is that the sand worm eats three enemies (or is killed), then it is destroyed, and then

void House::decrementUnits(ItemID_enum itemID) {

is called on Fremen (since sand worms are now Fremen), and they are not found to be alive, and as result the game declares Fremen to have been defeated.

The result seems to be cosmetic fortunately - there's just a silly ticker message happening. Game logic is not adversely affected.

I don't know what the right logic would be to fix though.

juj commented

This change does seem to do the trick.. maybe that might work:

diff --git a/src/House.cpp b/src/House.cpp
index 2105735b..6c7ef8c4 100644
--- a/src/House.cpp
+++ b/src/House.cpp
@@ -373,8 +373,13 @@ void House::decrementUnits(ItemID_enum itemID) {
         lossValue_ += context_.game.objectData.data[itemID][static_cast<int>(houseID_)].price;
     }

-    if (!isAlive())
-        lose();
+    // If this was the last unit of this house, mark this house lost (and trigger win/lose conditions)
+    // However if we were an AI controlled Sandworm (from Fremen NPC house), do not advertise Fremen
+    // house having lost.
+    if (!isAlive()) {
+        const bool silentLoss = ai_ && itemID == Unit_Sandworm;
+        lose(silentLoss);
+    }
 }

 void House::incrementStructures(ItemID_enum itemID) {

The Sardaukar drop troop reinforcements are a question mark however. I don't see any Sardaukar troops in any of my playthroughs, and I think when player kills the Sardaukar troops if they did come in, there should not be a message "House 'Sardaukar' has been defeated." . So a fix to this Fremen Sandworm scenario would be best to also make sure Sardaukar support troops won't have the same issue.