Skip to content

Monster respawn time is tenths of a second, not milliseconds - #2291

Open
denislauri1999 wants to merge 2 commits into
NosCoreIO:masterfrom
denislauri1999:pr/monster-respawn-unit
Open

Monster respawn time is tenths of a second, not milliseconds#2291
denislauri1999 wants to merge 2 commits into
NosCoreIO:masterfrom
denislauri1999:pr/monster-respawn-unit

Conversation

@denislauri1999

Copy link
Copy Markdown
Contributor

monster.dat, PREATT, fifth field. An ordinary wolf carries 400, which is forty seconds. Both call sites — MonsterRespawnHandler and CaptureService — read it as milliseconds, so every monster in the game came back in four tenths of a second: a hundred times too fast.

Nothing reported it. No exception, no log, just maps that never empty and a monster reappearing on top of whoever had just killed it — the kind of mistake only noticed by counting.

Two independent confirmations of the unit:

  • the atlagaming API exposes the same field as respTimeSek and gives 40.0 for that 400;
  • the older emulator divides it by ten.

The conversion moves into one place, RespawnTiming, because the two call sites had already drifted into two copies of the same arithmetic. It keeps the one-second floor that was there: some event monsters declare zero, and respawning them the instant they die would mean never letting them die.

Tests

Four, including the two that would pass a wrong implementation quietly — a declared time of zero, and a value large enough to overflow the multiplication if it is not done in 64 bits.

Zero warnings, all tests pass.

denislauri1999 and others added 2 commits August 23, 2026 16:32
…onds

monster.dat, PREATT, fifth field. An ordinary wolf carries 400, which is forty
seconds. Both call sites read it as milliseconds, so every monster in the game
came back in four tenths of a second - a hundred times too fast.

Nothing reported it: no exception, no log, just maps that never empty and a
monster reappearing on top of whoever had just killed it.

Two independent confirmations of the unit: the atlagaming API exposes the same
field as respTimeSek and gives 40.0 for that 400, and the older emulator
divides it by ten.

The conversion moves into one place, RespawnTiming, because the two call sites
had already drifted into two copies of the same arithmetic. It keeps the
one-second floor that was there: some event monsters declare zero, and
respawning them the instant they die means never letting them die.

Four tests, including the two that would pass a wrong implementation quietly -
a zero time, and a value large enough to overflow the multiplication if it is
not done in 64 bits.
The unit is the only non-obvious thing here; two lines cover it. The test's
name already says what a failure would mean.

Two files had also picked up a UTF-8 BOM, which showed as a whole-line diff
against master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 25 minutes.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 78847e54-fcbd-45f2-b46c-cc42d8efdc5d

📥 Commits

Reviewing files that changed from the base of the PR and between 55dc38c and 60c6a9f.

📒 Files selected for processing (4)
  • src/NosCore.GameObject/Messaging/Handlers/Battle/MonsterRespawnHandler.cs
  • src/NosCore.GameObject/Services/BattleService/CaptureService.cs
  • src/NosCore.GameObject/Services/BattleService/RespawnTiming.cs
  • test/NosCore.GameObject.Tests/Services/BattleService/RespawnTimingTests.cs

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

1 participant