Brutal-OXCE 9.1.4

[Alpha Centauri Bear]

Good observations. I thought about the same.

Few questions.

Is this setBattleGame an OXC/OXCE code? Why BAI suddenly makes it break? Do you speculate there is a trouble waiting to happen and BAI just create a condition for it to popup?

I understand PathfindingNode is application class and not the library one. Didn't understand a reference on "PathfindingNode usually has a problem when deleting ... ". Is it some common knowledge?

[Alpha Centauri Bear]

Another thing I don't understand why storing linked list in vector may cause any trouble. Links are stored as pointers. Meaning destructor will just delete the pointer itself disregarding pointed object altogether. It should be completely irrelevant if pointed object is deleted by that time.

[Yankes]

The _battleGame that is being deleted in SavedGame::setBattleGame looks like a valid object. It has members like _missionType = "STR_TERROR_MISSION", which is what the mission is. And the parameter when it's called from DebriefingState::init() is just a null-pointer. So it tries to delete the current valid _battleGame-object and then set the _battleGame-pointer to nullptr.

The issue really seems to be deleting the _nodes and _altNodes-vectors. (std::vector<PathfindingNode> _nodes, _altNodes;)

According to ChatGPT it could be problematic that the PathfindingNode-object has a pointer to another PandfindingNode via PathfindingNode* _prevNode;

"The PathfindingNode class seems to contain pointers (PathfindingNode* _prevNode) that reference other PathfindingNode instances, forming a linked structure. When you're using std::vector to manage a collection of PathfindingNode instances and they contain interconnections via pointers (_prevNode), deleting these nodes within a std::vector can potentially lead to issues.

If nodes are connected in a complex manner (such as forming a linked list or a graph), deleting them within a std::vector could break those connections and cause issues when pointers are no longer valid after deletion."

And how this could affect anything? do `PandfindingNode` try delete `_prevNode`? No, it could even work without any destructor.
ChatGPT blindly try guess some thing without no understating of part of this code.
I suggest small change, set `_missionType = "F*** UP";` in destructor and put break point before that, and see if any break point stop, object do not have this value already there. You slimly do not rule out possibility of double delete of this object. If object is garbage it still hold data, whole point is normally you can leave trash behind if nobody can notice, and in case of string it can still hold value as `std::string` is allowed to have buffer inside itself for charters. Alterative it still point to old memory that had text. And in case of `std::vector` it could be this second case.

[Xilmi]

Good observations. I thought about the same.

Few questions.

Is this setBattleGame an OXC/OXCE code? Why BAI suddenly makes it break? Do you speculate there is a trouble waiting to happen and BAI just create a condition for it to popup?

I understand PathfindingNode is application class and not the library one. Didn't understand a reference on "PathfindingNode usually has a problem when deleting ... ". Is it some common knowledge?

As far as I know that's OXC-code. OXCE-team isn't fond of manual memory-management but hasn't replaced it everywhere due to how enormous of a task that is.

If I knew why BAI makes it break, I'd already have fixed it. There are some massive differences in how BAI uses pathfinding though, so it's not surprising that it has an impact on that.

Notably: I've split findReachable into two methods. The new one being findReachablePathFindingNodes. Old findReachable created all that valuable information for the AI but then only returned a vector of Positions. findReachablePathFindingNodes returns the vector of Nodes and the Nodes have a lot of Information that are useful to AI. For example I can determine the TU cost to reach any tile instantly without another path-finding-call which makes BAI several orders of magnitude more efficient in finding tiles it could go to. VAI just limited itself to a random selection of 111 tiles for that because it ran a separate pathfinding for all of those.
So eventually I didn't just run it for the current unit but also for other units. And not only for the unit's real location but also for hypothetical locations. This is what allows the creation of the move-order, a really well working threat-map and an algorithm that can estimate whether a unit can rely on the fire-support of it's friends.
Since the _nodes get changed by each path-finding-call and I needed them to remain what they are, I created _altNodes, which is for all the calls for other units. The result of that is also buffered in the units itself, so I don't need to call pathfinding for other units that can't have moved anyways.

One idea I had that maybe one of the many calls of findReachablePathFindingNodes that use a hypothetical starting-location gives it an invalid starting-location, which then messes up nodes in a way that causes an issue when trying to delete them. What I haven't done yet is to sift through all the 10,000 _nodes and _altNodes to look at whether something looks fishy there. If I, for example call pathfinding for an invalid position something bad could happen. So yeah, there's definitely enough difference in BAI compared to VAI when it comes to the usage of Pathfinding to warrant causing an issue that never happened with VAI.

I don't think there "usually" is a problem when deleting. Even in BAI this is a rare and hard-to-reproduce case. I just so happened to have a save now where it happens reproducibly. I don't know what about that save is special so it happens there but not in many other scenarios.

[Xilmi]

Another thing I don't understand why storing linked list in vector may cause any trouble. Links are stored as pointers. Meaning destructor will just delete the pointer itself disregarding pointed object altogether. It should be completely irrelevant if pointed object is deleted by that time.

As Yankes said: ChatGPT is a devious assistant. It can make up theories that sound kinda plausible, even if they are nonsense. Especially if you speculate yourself. It will pick up on your speculation and come up with a plausible-sounding explanation as for why something where you just asked: "Could it be that...?" is the case.
It sometimes does help to find the real cause. But in other cases it can put you on a wrong path for a long time. I need to be more careful about how I use it.

[Xilmi]

I suggest small change, set `_missionType = "F*** UP";` in destructor and put break point before that, and see if any break point stop, object do not have this value already there. You slimly do not rule out possibility of double delete of this object. If object is garbage it still hold data, whole point is normally you can leave trash behind if nobody can notice, and in case of string it can still hold value as `std::string` is allowed to have buffer inside itself for charters. Alterative it still point to old memory that had text. And in case of `std::vector` it could be this second case.

I've worked with break-points and I'm pretty sure it's not a double-deletion because it happens after reaching the destructor for the first time.
Also it quickly jumped into libraries I didn't have the sources for while trying to debug with break-points.
Basically it gets to:
Pathfinding::~Pathfinding()
{
}
as the last part of "my code" and then disappears into the destructor of vector, where I can no longer see what's going on.

My new theory is that something in the _altNodes is corrupt. Possibly because I dereference a Position-pointer to determine a starting-position. If this pointer is not null but something that is invalid, I don't know what effect that could have. So the next thing I'd like to investigate is whether this could have happened. The logic that checked for adjacent positions to a unit for meeles to see what side had a weakness used to produce out-of-bounds positions. THis was only found by accident, when I had a case where an AI unit did nothing. Turned out it wanted to go to a position outside of the map to attack but obviously couldn't do so. And while that particular issue was fixed, I'm thinking that maybe I'm trying to do a path-finding-call with a similar position like that.

[Alpha Centauri Bear]

I found one attempt to set position (-1, 0, 0). Not sure if this is the culprit but it is definitely a dirty code that scratches my eye.

BattleUnit.h

Code: [Select]

int _tileLastSpottedForBlindShotByHostile, _tileLastSpottedForBlindShotByNeutral, _tileLastSpottedForBlindShotByPlayer = -1;

AIModule.cpp

Code: [Select]

std::map<Position, int, PositionComparator> AIModule::getReachableBy(BattleUnit* unit, bool& ranOutOfTUs, bool forceRecalc, bool useMaxTUs)
{
Position startPosition = _save->getTileCoords(unit->getTileLastSpotted(_unit->getFaction()));

With last spottet tile == -1 it sets startPosition to (-1,0,0), which is incorrect, as it does not fit into map tile vector, obviously.

You should treat it as "not spotted" and accurately wrap around it.

[Alpha Centauri Bear]

Simply adding this check at the top of this function makes this not to crash anymore. I am not sure if it also screwed the AI computation but, I guess, you can figure the rest.

Code: [Select]

std::map<Position, int, PositionComparator> AIModule::getReachableBy(BattleUnit* unit, bool& ranOutOfTUs, bool forceRecalc, bool useMaxTUs)
{
std::map<Position, int, PositionComparator> tuAtPositionMapDefault;

if (unit->getTileLastSpotted(_unit->getFaction()) == -1)
return tuAtPositionMapDefault;
...
Position startPosition = _save->getTileCoords(unit->getTileLastSpotted(_unit->getFaction()));


[Yankes]

I tried last version from git and have this crash, now I do not get crash in battle destructor. when I try quit game after it to try again, game crash on `Globe` destructor.
This look like big memory corruption.

[ps]

After changing some mods (last time I had some test mod enabled) game crash on `ctrl-a` and code at `PathfindingNode::connect`.
Probably Alpha is right ablaut cause of this error.

[Xilmi]

Simply adding this check at the top of this function makes this not to crash anymore. I am not sure if it also screwed the AI computation but, I guess, you can figure the rest.

Awesome! Thanks a lot for finding it!

Also now everything makes sense!

Why it was happening in terror-missions but not regular ones.

The reason for that is that "friendReachable" is populated before assigning the position-assumptions. I didn't think about that Neutrals are friends to the player and player-units are friends to the neutrals.

So the AI doesn't know their real location and the code that assumes the positions for the enemies is run between filling "friendReachable" and "enemyReachable", so it didn't happen there because the enemy-units didn't have the -1 initial-position anymore.

This also means that using autoplay or brutal AI for neutrals was a condition for it to happen, hence not everyone ran into this issue. Another condition must've been for them to not meet before the mission ends.

So, yeah, simply ignoring the units who we are allied with and don't know their location yet definitely is a sufficient and side-effect-free-solution.

Edit: Okay, I now also tested it and as expected it no longer crashes. What is odd, though is that it impacts how the aliens behave. They seem to move in a different order and do different things. They probably don't have a "TileLastSpotted" for themselves. But in the case it's a unit of the own team it uses the actual tile and not the last-spotted one. So your solution isn't entirely correct yet and I need to slightly modify it.

[Alpha Centauri Bear]

No. I didn't hope it will be correct. This is just an illustration that you should handle -1 value separately and not let it to main processing branch. A lot of IFs everywhere, etc.

I usually rename my variable so the code breaks everywhere it is used. This way I can cover all the usage.
Also, it may be cleaner to hide it under extra layer. Say instead of Position introduce something like OptionalPosition that may either contain valid position or no position. Extra methods: bool hasValue(), Position getValue(). Latter should blow if there is no value. Stuff like this, you know.

[Xilmi]

Coming back to replying to the "normal" kinds of posts here after that horrifying excursion to the memory-corruption-bug.

Yeah, I can. This previous savefile I attached is the mission that I tried multiple times with various aggro settings. The 7.13.1 where I set recommended 1/2 "baseline" + modder-set squashed my units in 10 turns, just because every enemy attacked simultaneously, minotaurs rushed with macro-flamers and axes (actually, I love how BAI prefers macro-flamers instead of melee option). Also, BAI thrown more grenades. I was lacking ifrepower and had to decide whether I should fire or reserve TU's for incoming enemies. None would help, because it's map with 50 enemies.

I actually sat down and played that map myself just now with 8.0.4. Took me a good hour. I lost while killing 27 enemies in 20 turns. But admitedly I played a bit poorly in regards to the proxy-grenades. I forgot my soldiers had them and repeatedly stepped into my own mines.

This mission has tons of cover so I can very well imagine that the AI can do a good job with being aggressive on it. They also threw lots and lots of grenades. The high health-pools compared to the damage and the availability of medikits also helped me under consideration that the enemy played less aggressively. It meant I actually could make use of the medikits.
What also was extremely helpful were the civilians. They were "tanking" a lot of the minotaur-attacks. Most of the minotaurs died early after exposing themselves by attacking civilians. Had the civilians not been and I the target of their attacks I'd have had much higher losses early on, I suppose.

Overall the level of challenge felt good considering me probably not playing very well.
 

Well, that mission was pain in the ass (I dealt it 4 times, lost badly 3 times and easily won on 4th). It usually spawns in mountains/open field, but now spawned in close urban area. That is why sniper-rifle loadout was wrong. I clearly don't wan't to play it again to check.

You mean you didn't replay the same mission but only a similar one? The one you won was the one in the urban area or not? It's not entirely clear to me.

But clearly not in case of battles vs x3 enemies (of same power), where rushing strategy for AI is better.

A TFTD-player told me that for his play-style it is very helpful if the AI rushes. I think the main difference between the scenarios is weapon-range. In vanilla-TFTD all weapons have infinite range. When there's more units with short range, the entire team probably should play more aggressively.

My idea is that the AI should determine the proper approach based on situational in-game-logic. Another important point is decisiveness. So some units hanging back while others try to push and overall it creates a trickling-in-scenario is definitely not the right play.

The thing angered me was when BAI in two turns thrown 5 units through the same door, which had exit tile in reaction-sight of my snipers. That was enough to get that something goes wrong with decision making.

Okay, this really sounds like it could be related to the bug fixed in 8.0.2:
"Fixed an issue with the spotter-determining-code that caused the spotter to think their friends would always have enough weapon-range."
This probably caused them to nominate one spotter after another thinking other units had it's back when they didn't.

Also: I know you like Ironman but when it comes to producing saves that showcase a buggy behavior, that's not useful as Ironman doesn't make rotating autosaves, which are very helpful to catch buggy behavior. You can turn off Ironman by simply manually editing the save-file. I do that every time with your saves so I can test properly.

Well, the 4 for max aggressiveness is arbitrary number to which everyone got used to. And it was introduced by you.
I think simpler is better. So, if you chose this over min-max range, no trouble.
If min-max in same battle, then bunch of attackers, bunch of lurkers and so on. Which will result in force split. Which, in its turn, will lead to lose of competitive power. In theory. I don't know what will happen during real gameplay, given all these advances in BAI decision making.

In a way that "fluid"-aggressiveness-scaling seems like a bad approach when it comes to decisiveness. I put autoplay to maximum because anything else ruins the comparability of my benchmark-saves. But I see sometimes that this kind of approach works well enough. I have an idea for something that is kinda similar to maximum but that situationally considers cover when the unit made contact and it's readily available.

I'm thinking that situationally recognizing whether to perform this behavior or the current one is probably a better goal than indecisiveness caused by aggession-fluidity. Think about it in a WW1-Trench-war-scenario. You either stay in your trench or you try to storm the enemie's trench. There's no in-between, where you slowly inch-forward towards their trench or some of your side stay in the trench while the others rush forwards.
 

Yep. I am going to make a submod, so players don't have to do anything.
I'll do it right ahead after getting the understanding of actual/final decision weights.
So that would be great to chat on them and/or look at formulas, if you share the lines where to look at and how to understand them.
But then, approach should be verified, at least not drastically changed every new version.

My convictions are very volatile. Like I'm very quick to change my mind. I'd say at least in the current situtaion there isn't too much benefit for setting all values in a sub-mod, while I'm thinking about drastic-changes to the aggession-model.

The formula is pretty much this:

100 / (discoverThreat + walkToDist * myAggressiveness);

discoverThreat is measured in TU. It basically means: "From where I think the enemies are. If I move to that tile, how many combined TUs will the enemies have from where they could spot me.
walkToDist is the distance of the reachable tile that is closest to the enemy + the max-TU of the unit.

As you can see, the more aggressive, the bigger the impact of the distance compared to the discover-threat. If discoverthreat and aggression both are 0 then it will use 100 / walkToDist;

UPD don't take my whining about "AI is too easy" and "AI is too strong" as bipolar disorder.
I am looking for balance that kicks my ass, but yet is possible to beat:)

I think my reaction to "too strong" in the future will be: "Then mod it easier/lower the difficulty", whereas I will take to "too easy" very seariously and will want to look into the reasons for that and what to do about it.

[Juku121]

I think my reaction to "too strong" in the future will be: "Then mod it easier/lower the difficulty", whereas I will take to "too easy" very seariously and will want to look into the reasons for that and what to do about it.

All right, I'm seeing that BAI is now starting to get perilously close to the 'auto-spot on hit' mechanics of vanilla scout-sniper via spotting on accidental hit and spotting firing locations not approximately, but with extreme precision. Which matters most on reaction fire, but even for usual combat it means that the already troubled camo/vision system is being sidelined in favour of straight-up firefights. While I don't dislike the concept, I very much do dislike the execution. How do I 'mod it easier'? Difficulty will do nothing for this issue, and I don't really want to just plain handicap every enemy weapon for beyond-LoS firing.

[Xilmi]

All right, I'm seeing that BAI is now starting to get perilously close to the 'auto-spot on hit' mechanics of vanilla scout-sniper via spotting on accidental hit and spotting firing locations not approximately, but with extreme precision. Which matters most on reaction fire, but even for usual combat it means that the already troubled camo/vision system is being sidelined in favour of straight-up firefights. While I don't dislike the concept, I very much do dislike the execution. How do I 'mod it easier'? Difficulty will do nothing for this issue, and I don't really want to just plain handicap every enemy weapon for beyond-LoS firing.

Brutal AI has worked like this since quite some time.
I think this is valid criticism and I think I can change it and/or make it optional.

I could just do the same thing that I did with doors when I still had the AI notice door-opening:

The AI considers a unit as a valid target to attack on the current turn when the unit was "seen" in the current turn. target->getTurnsSinceSeen(_unit->getFaction()) == 0

"updateEnemyKnowledge" sets this value to 0 when a unit shoots (via reaction-fire) or gets hit (via auto-spot on hit).

Changing it so that in these cases the position is updated but not the "turnsSinceSeen", would mean the unit would no longer be attackable without actually being seen. The AI would first have to confirm the position by walking in on the unit.

I think this would be justifiyable to have as an option. It's a bit of a grey area in terms of whether that can be considered cheating or not. The introduction of holding Alt to see where the AI units were spotted shooting from even without actually seeing the units leveled the playing-field when it comes to precision. But that could also be considered as: "Allowing the player to cheat too so the AI-cheat becomes more justifyable."

So I'd say I'll make it an option and put it under battle-scape and not AI so that whether it's enabled or not it's same for both sides.

[Juku121]

That would work.

I also don't dislike the idea itself, just how accurate and long-range the spotting is. Several games have had something similar (though I don't know if the AI had access to it, too), like UFO: ET and Silent Storm displaying 'heard' enemies or the alien screams of nuCom.

I don't know how one could make this less precise while not neutering AI decision-making. Some ideas: 1) mark a 3x3 or even larger area, make the AI consider using only HE weapons/grenades/waypoint weapons (edit: that is, spray-waypoint autofire weapons, not Blasterkin /edit) on these. 2) Mark only a random location in that same 3x3, 5x5 or whatever area. 3) My old idea of introducing an additional "no LoS and no spotter' malus, i.e. if a unit (yours or the enemy) doesn't have LoS on their target, their accuracy is multiplied by a fraction, typically 50% (OXCE functionality); if they additionally don't have a 'spotter' (either ruleset or just BAI-enabled unit), multiply the chance by another penalty, like another 50%. So real alien snipers and grenade chuckers can still do this, but not everyone all the time.