diasurgical
Prevent monsters from standing idle in feared element
This allows monsters to treat tiles with feared elements as safe, if the monster is already standing in a tile with the feared element.
I chose this route as opposed to reworking monster AIs, since I believe changing the AI of monsters to allow them to behave differently (such as walking away from the player when this previously was never an option for them), is against the scope of the project. This change does not put the monster in more peril than it previously was, and adds difficulty for the player.
Right below your new check, there is an existing one that is very similar: the difference being it seems to use a different position variable.
Can you elaborate on the difference between the 2?
And if your code is indeed a different situation, I'd suggest trying to repurpose your logic there and reuse it on the last check since it is checking the exact same thing.
The initial check using monster.position.tile determines if the monster is currently standing in a dangerous tile (i.e., fire or lightning). This is important to understand the monster's current situation and decide its next move.
The subsequent checks using position evaluate the safety of a potential new tile the monster might move to. This is crucial for decision-making regarding the monster's next move based on its current situation and the destination tile's properties.
The initial check using
monster.position.tiledetermines if the monster is currently standing in a dangerous tile (i.e., fire or lightning). This is important to understand the monster's current situation and decide its next move.The subsequent checks using position evaluate the safety of a potential new tile the monster might move to. This is crucial for decision-making regarding the monster's next move based on its current situation and the destination tile's properties.
In that case, I think your code breaks the original intent of the method, which is to check "whether a provided tile position is safe for a given monster". The moment you start checking another tile in the implementation (the one the monster is currently standing), you are doing something else.
I'd suggest this should not be done inside this method, or you'd have to at least reword the method to change its original intent.
Something I'd consider here is calling this same method 2 times: one for the "current" monster tile, and another time for the "target" tile. One would have to see if the other checks being performed there still make sense for both situations though.
Either way, I strongly feel like you are doing this logic in the wrong place there.
The initial check using
monster.position.tiledetermines if the monster is currently standing in a dangerous tile (i.e., fire or lightning). This is important to understand the monster's current situation and decide its next move.The subsequent checks using position evaluate the safety of a potential new tile the monster might move to. This is crucial for decision-making regarding the monster's next move based on its current situation and the destination tile's properties.
In that case, I think your code breaks the original intent of the method, which is to check "whether a provided tile position is safe for a given monster". The moment you start checking another tile in the implementation (the one the monster is currently standing), you are doing something else.
I'd suggest this should not be done inside this method, or you'd have to at least reword the method to change its original intent.
Something I'd consider here is calling this same method 2 times: one for the "current" monster tile, and another time for the "target" tile. One would have to see if the other checks being performed there still make sense for both situations though.
Either way, I strongly feel like you are doing this logic in the wrong place there.
Sorry, I have no idea what you are trying to say
Imagine you had a method BuyHouse, then you went and added code inside it to purchase a car. That's the idea.
You are adding tile-checking logic to a method that was only ever supposed to check the tile passed as parameter, against a different tile.
I think this PR is a good thing for the AI logic and avoids some crazy stuff where a monster is scared to move forward, because there is a firewall but it already stands in a firewall. 🙄 So personally I think this is a good addition/bugfix.
But I also have to say from a logic/naming perspective @julealgon has a good point.
The function IsTileSafe has this comment
* @brief Check if a tile is affected by a spell we are vunerable to
and I would expect the function to return this result.
Currently we have two places where IsTileSafe is called.
Perhaps we could change these two places to something like this:
if (IsTileSafe(monster, position))
return true;
// If our current tile is not safe then the new tile doesn't need to be "safer". This avoids that monster can't walk out of a firewall.
return !IsTileSafe(monster, monster.position.tile);
Just a suggestion. 🙂
