fix #4077 (monster id 0 in onSpawn event)

[Zbizu]

• [x] I have followed proper The Forgotten Server code styling.
• [x] I have read and understood the contribution guidelines before making this PR.
• [x] I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.

Changes Proposed

set monster ID before calling the script Issues addressed: #4077

[nekiro]

This event is called too early, with that fix you are setting id twice, It will work cause setId has id == 0 check, but anyway that's workaround and is probably problematic

[conde2]

Can't we just call setId on each creature class consctructor?

[nekiro]

Can't we just call setId on each creature class consctructor?

I believe it sets the id after creature is placed to avoid waste of ids for unplaced creatures, this is though very rare, when creature is created but not placed, so your suggestion might be actually quite good, unless I missed something

[Zbizu]

I believe it sets the id after creature is placed to avoid waste of ids for unplaced creatures, this is though very rare, when creature is created but not placed, so your suggestion might be actually quite good, unless I missed something

the id pool is big enough to run the server for several months without restarting, even on high exp ue spam server the pool will never get exhausted if you restart server daily

[Codinablack]

Would it not be best, for the end user, if "onSpawn" was a void, with the whole created monster passed after creation, and then there be another event in first SpawnMonster call, so earlier in execution, before deciding to spawn or not, to serve as the bool, which could have more information such as, onPrepareSpawn(monsterType, spawnId, spawnBlock, centerPos, radius, #aliveInSpawn, startup)

would work in a similar manner to onDeath, and onPrepareDeath

We could actually even keep onSpawn as a bool and have it remove the monster if false to keep from breaking backwards compatibility, as was suggested by forgee, and it still be called in same place for Game.CreateMonster().

[ramon-bernardo]

Simplest solution. Good!

@Zbizu the id pool is big enough to run the server for several months without restarting, even on high exp ue spam server the pool will never get exhausted if you restart server daily

I agree.

Any reason not to merge?

[Zbizu]

@ramon-bernardo

Any reason not to merge?

Divided opinions on the solution.

I have this code in my engine and it does the job so I won't be working on this pr anymore. If you guys are not interested in merging it in its current state, you can close it.

For anyone interested in continuing this pr: --MonsterAutoId could be called on failing to place monster to free the unused slot, though I'm not sure if that doesn't introduce a data race.

[ranisalt]

If no one is willing to dedicate time, I'd rather have the bug fixed.