movement: generalize; use vectors #16

Merged
NotAShelf merged 6 commits from SquirrelModeler/rogged:generalize-movement into main 2026-04-09 14:11:47 +00:00
Showing only changes of commit 7905ace2ca - Show all commits

fix: move action setter outside null guard

Squirrel Modeller 2026-04-09 15:22:50 +02:00
No known key found for this signature in database
GPG key ID: C9FBA7B8C387BF70

View file

@ -448,8 +448,8 @@ static int handle_movement_input(GameState *gs) {
if (result == MOVE_RESULT_MOVED) {
if (target != NULL) {
player_on_move(&gs->player);
action = 1;
}
SquirrelModeler marked this conversation as resolved Outdated

You should probably guard against potential null target here before attacking.

Because if try_move_entity returns MOVE_RESULT_BLOCKED_ENEMY but player_find_enemy_at fails to find the enemy (be it due to differing lookup logic, or if the enemy was killed by effects earlier in the turn), target would be NULL, and thus lead to an ub in player_attack. Something like:

   } else if (result == MOVE_RESULT_BLOCKED_ENEMY) {
     target = player_find_enemy_at(gs->enemies, gs->enemy_count, new_x, new_y);
-    player_attack(&gs->player, target);
-    action = 1;
+    if (target != NULL) {
+      player_attack(&gs->player, target);
+      action = 1;
+    }
   }
You should probably guard against potential null target here before attacking. Because if `try_move_entity` returns `MOVE_RESULT_BLOCKED_ENEMY` but `player_find_enemy_at` fails to find the enemy (be it due to differing lookup logic, or if the enemy was killed by effects earlier in the turn), target would be `NULL`, and thus lead to an ub in `player_attack`. Something like: ```diff } else if (result == MOVE_RESULT_BLOCKED_ENEMY) { target = player_find_enemy_at(gs->enemies, gs->enemy_count, new_x, new_y); - player_attack(&gs->player, target); - action = 1; + if (target != NULL) { + player_attack(&gs->player, target); + action = 1; + } } ```

If we move the action = 1 into the null guarded if statement, then enemy AI breaks. We can do:

if (target != NULL) {
  player_attack(&gs->player, target);
}
action = 1;

Which fixes that bug.

Ignore this. I added an incorrect guard.

~~If we move the action = 1 into the null guarded if statement, then enemy AI breaks. We can do:~~ ``` if (target != NULL) { player_attack(&gs->player, target); } action = 1; ``` ~~Which fixes that bug.~~ Ignore this. I added an incorrect guard.
action = 1;
} else if (result == MOVE_RESULT_BLOCKED_ENEMY) {
target = player_find_enemy_at(gs->enemies, gs->enemy_count, new_x, new_y);
player_attack(&gs->player, target);