movement: generalize; use vectors #16

SquirrelModeler · 2026-04-09T12:34:17Z

SquirrelModeler commented

2026-04-09 12:34:17 +00:00

Generalized movement, so that all entities move the same.

SquirrelModeler added 1 commit

2026-04-09 12:34:17 +00:00

refactor(movement): generalize movement and use vectors d01a54161d

NotAShelf changed title from ~~refactor(movement): generalize movement and use vectors~~ to movement: generalize; use vectors

2026-04-09 12:46:31 +00:00

NotAShelf requested changes

2026-04-09 13:02:10 +00:00

Dismissed

NotAShelf left a comment

Two minor comments. The former is a nitpick but I'd still like for it to be gotten out of our way since it's a trivial change

src/main.c Outdated

					
				@ -444,0 +448,4 @@

				  if (result == MOVE_RESULT_MOVED) {

				    player_on_move(&gs->player);

				    action = 1;

				  } else if (result == MOVE_RESULT_BLOCKED_ENEMY) {

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.

SquirrelModeler marked this conversation as resolved

src/player.h Outdated

					
				@ -8,3 +8,2 @@

				// Move player to (x+dx, y+dy); returns 1 if moved, 0 if blocked

				int player_move(Player *p, int dx, int dy, Map *map);

				// Apply status effect, healing ect

Typo, should be "healing, etc."

SquirrelModeler marked this conversation as resolved

SquirrelModeler added 3 commits

2026-04-09 13:16:34 +00:00

fix: typo, remove old method 5e2f752c7e

chore: remove unused import 62a4442844

fix: null check guard against target 0cb0ca17ff

SquirrelModeler added 1 commit

2026-04-09 13:23:41 +00:00

fix: move action setter outside null guard 7905ace2ca

SquirrelModeler changed title from ~~movement: generalize; use vectors~~ to WIP: movement: generalize; use vectors

2026-04-09 13:25:55 +00:00

SquirrelModeler commented

2026-04-09 13:26:21 +00:00

Currently there is a bug with healing. Putting this into WIP until I've fixed it.

SquirrelModeler commented

2026-04-09 13:27:14 +00:00

I am stupid. I added an incorrect guard. Fixing it now.

SquirrelModeler added 1 commit

2026-04-09 13:29:27 +00:00

fix: move guard check 4044543fe1

I am stupid. I added the guard on the wrong check. Ffs...

SquirrelModeler changed title from ~~WIP: movement: generalize; use vectors~~ to movement: generalize; use vectors

2026-04-09 13:30:10 +00:00

NotAShelf approved these changes

2026-04-09 14:11:34 +00:00

NotAShelf merged commit 4dfe52ae72 into main

2026-04-09 14:11:47 +00:00

NotAShelf referenced this pull request from a commit

2026-04-09 14:11:49 +00:00

movement: generalize; use vectors (#16)

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

2 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

NotAShelf/rogged!16

Rows
Columns