Skip to content

Fix potential TOC/TOU issues caused by dealing damage between can_move_to and actually moving - #135

Merged
SmallJoker merged 2 commits into
minetest-mods:masterfrom
j-r:damage_after_move
Aug 28, 2026
Merged

Fix potential TOC/TOU issues caused by dealing damage between can_move_to and actually moving#135
SmallJoker merged 2 commits into
minetest-mods:masterfrom
j-r:damage_after_move

Conversation

@j-r

@j-r j-r commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Damaging creatures can invoke a lot of code that may change the world, e.g. killing a player may place a bone block that is then potentially overwritten by the digtron nodes themselves.

This PR moves the creature damage to after moving the digtron to minimize the potential for TOC/TOU issues.

It also includes a separate commit with a small not directly related change to annotate node class in the message warning about missing damage_creatures callback.

j-r added 2 commits August 18, 2026 15:42
Dealing damage involves a lot of callbacks that may modify the map enough
to invalidate the validation done before moving (e.g. killing a player
may place a bones block in the digtrons way).

Moving first minimizes the chance of the map changing between validation
and movement.
@SmallJoker

SmallJoker commented Aug 22, 2026

Copy link
Copy Markdown
Member

What if the creature spawns a node? In that case we'd have to check twice and move in the end. However, I'm afraid there's no perfect solution.
EDIT: Sorry, wrong key combination.

@SmallJoker SmallJoker closed this Aug 22, 2026
@SmallJoker SmallJoker reopened this Aug 22, 2026
@j-r

j-r commented Aug 23, 2026

Copy link
Copy Markdown
Contributor Author

What if the creature spawns a node? In that case we'd have to check twice and move in the end. However, I'm afraid there's no perfect solution.

EDIT: I think I understand your concern now. If the creature now places some blocks into the digtron after the digtron moved, I think that's fine, because the creature's code does it after checking the map, therefore intentionally.

@SmallJoker

Copy link
Copy Markdown
Member

Okay. Seems fine then. Will merge in a few days unless there are objections.

@SmallJoker
SmallJoker merged commit 2453227 into minetest-mods:master Aug 28, 2026
2 checks passed
@j-r
j-r deleted the damage_after_move branch August 28, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants