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 4044543fe1 - Show all commits

fix: move guard check

I am stupid. I added the guard on the wrong check. Ffs...
Squirrel Modeller 2026-04-09 15:28:57 +02:00
No known key found for this signature in database
GPG key ID: C9FBA7B8C387BF70

View file

@ -446,14 +446,14 @@ static int handle_movement_input(GameState *gs) {
MoveResult result =
try_move_entity(&gs->player.position, direction, &gs->map, &gs->player, gs->enemies, gs->enemy_count, true);
if (result == MOVE_RESULT_MOVED) {
if (target != NULL) {
player_on_move(&gs->player);
}
player_on_move(&gs->player);
action = 1;
} else if (result == MOVE_RESULT_BLOCKED_ENEMY) {
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.
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 (action)