Crash using reload in spells

[Peixonauta2]

Describe the bug

The "/reload spell" command can cause server crash. This happens because monsters will begin to cast random spells and if one of those random spells require a player structure, the server will crash. This bug also happens if using revscripts spells and then reloading scripts.

How to Reproduce?

Set a monster to use any registered spell.
NOTE: The spell needs to be a registered spell (spells.xml or revscript spell) and not a configured spell at monster's file.
If the monster is using a register spell, such as sudden death rune, for example:
{name ="sudden death", interval = 2000, chance = 16, minDamage = -450, maxDamage = -550, range = 7, target = true}
It will begin to cast another spell.
If the monster is using a configured spell, such as a recreation of sudden death rune, for example:
{name ="combat", interval = 2000, chance = 16, type = COMBAT_PHYSICALDAMAGE, minDamage = -450, maxDamage = -550, range = 7, shootEffect = CONST_ANI_SUDDENDEATH, effect = CONST_ME_MORTAREA, target = true}
It will not cast another spell.

Give us all details to reproduce the behavior:

1. Make sure that the monster that you are using to test it have a registered spell
2. Use /reload spell (if using spells.xml to register the spell) or /reload scripts (if using revscripts to register the spell)
3. The monster will begin to cast another spell
4. Crash if the random spell requires a player structure

Screenshots

First, the monster is using sudden death rune, as it should (before using /reload):

After using /reload scripts:

Different spell than sudden death, which is the one registered:

Environment

• Windows 10
• Last commit

Additional context

Found a way to "fix" it, but I don't really know if it is effective: reload monsters scripts after reloading scripts on game.cpp

(Sorry, I'm new on github and don't really know how to use it properly, just want to contribute a little.)

[beats-dh]

So when you use the /reload scripts command will it reload monsters too?

[Peixonauta2]

It's dumb, I know but since I'm no C++ expert, I didn't find another way to resolve it. I'm not suggesting this as a solution, just reporting what I did to be able to reload scripts without getting all monsters crazy.

[beats-dh]

It's dumb, I know but since I'm no C++ expert, I didn't find another way to resolve it. I'm not suggesting this as a solution, just reporting what I did to be able to reload scripts without getting all monsters crazy.

It's not stupid, it was a good idea. Did you notice any crash or choke on the reload?

[Peixonauta2]

It's dumb, I know but since I'm no C++ expert, I didn't find another way to resolve it. I'm not suggesting this as a solution, just reporting what I did to be able to reload scripts without getting all monsters crazy.

It's not stupid, it was a good idea. Did you notice any crash or choke on the reload?

No, actually the reload execution was fast and the crashes stopped. I recommend more people to test it.
Do you think that this has anything to do with revscripts monsters? I don't remember of this problem happening before when monsters used to be registered as xml.

[beats-dh]

It's dumb, I know but since I'm no C++ expert, I didn't find another way to resolve it. I'm not suggesting this as a solution, just reporting what I did to be able to reload scripts without getting all monsters crazy.

It's not stupid, it was a good idea. Did you notice any crash or choke on the reload?

No, actually the reload execution was fast and the crashes stopped. I recommend more people to test it.
Do you think that this has anything to do with revscripts monsters? I don't remember of this problem happening before when monsters used to be registered as xml.

It may have to do with monster changes to rev, you can test this modification along with the PR of spells in rev opentibiabr/otservbr-global-archived#2588.

[majestyotbr]

So when you use the /reload scripts command will it reload monsters too?

No, the /reload scripts just update the scripts, to reload the monsters you should use the command /reload monsters because the monsters are still in data/monster, they are not in data/scripts.

[majestyotbr]

Remember, the command /reload should be used only for tests.

[majestyotbr]

/reload scripts
/reload monsters

[Peixonauta2]

Remember, the command /reload should be used only for tests.

Agree with this. Just reported because it may be annoying to get your tests to crash.

[github-actions]

This issue is stale because it has been open 120 days with no activity.

[dudantas]

Your issue will be transferred to the canary repository

[github-actions]

This issue is stale because it has been open 120 days with no activity.

[andersonfaaria]

I actually liked the work-around to ask to reload monsters upon reloading scripts. It's clearly a depency that can't be ignored.