server icon indicating copy to clipboard operation
server copied to clipboard

onMonsterMagicPrepare should be refactored and the column removed from existance

Open TeoTwawki opened this issue 4 years ago • 13 comments

  • [x] I have paid attention to this example and will edit again if need be to not break the formatting, or I will be ignored
  • [x] I have searched existing issues to see if the issue has already been opened
  • [x] I have checked the commit log to see if the issue has been resolved since my server was last updated
  • [x] I have read the Contributing Guide
  • [x] I have specified what branch this happens on branch: all

Additional Information (Steps to reproduce/Expected behavior) : Currently, if the scripted spells flag in mob_pools is set, the mobs spell list is completely ignored and whatever you set in onMonsterMagicPrepare() gets used instead. if it is missing from the script, your monster will not cast.

If the flag is NOT set, then nothing you put in onMonsterMagicPrepare gets used at all.

Neither of these behaviors is desirable, they both lead to users getting confused, and instead the spell list should be used unless we use the script to explicitly override what is being cast.

As it stands now you cannot use the function to trigger special behavior unless you want to handle the mobs entire set of available spells via a set of math.random() conditionals and that blows.

TeoTwawki avatar Oct 14 '21 22:10 TeoTwawki

This seems like it should follow more of a fallback scenario. If Prepare, then use it, else default to other behavior

claywar avatar Oct 14 '21 22:10 claywar

I want to do some thing in this function other than changing what spell is being cast. And we shouldn't need a db column to on off this at all. If the script said replace the spell, do it, if it didn't or the function isn't even used, don't. Pretty simple.

TeoTwawki avatar Oct 14 '21 22:10 TeoTwawki

What monsters have AI requiring this? Also, could "absorbs magic damage / ice/shock/etc spikes while casting" be added to this function somehow? Retail NMs have this behavior a lot.

ShiyoKozuki avatar Oct 15 '21 17:10 ShiyoKozuki

2 completely different things. Anyway, this was found (resurfaced, to be precise, since it was known at some point) while testing Lord of Onzozo.

Xaver-DaRed avatar Oct 15 '21 17:10 Xaver-DaRed

2 completely different things. Anyway, this was found (resurfaced, to be precise, since it was known at some point) while testing Lord of Onzozo.

How is it? When the mob starts to prepare magic it should get absorb x or spike effects, I assume that's how SE coded it. Unless you can already do that.

ShiyoKozuki avatar Oct 15 '21 18:10 ShiyoKozuki

We are talking about how mobs should fall back to spell list if they have no onMonsterMagicPrepare() in mob script, and how having to set it in database wich one it uses shouldnt be needed. You are talking about a modifier/effect, wich should be coded in its appropiate place.

As I said, 2 completely diferent things.

Xaver-DaRed avatar Oct 15 '21 19:10 Xaver-DaRed

The function, as it exists currently only is run when the database value is set to 1 and it simply returns a number which is then used to replace the spell ID. So current usage is to do some random-ing to decide a spell id and then return it. This all kinda sucks.

Desired behavior is run this function if it exists in the script anytime a spell is being prepared before cast begins, letting you do whatever before casting and it doesn’t do anything during casting either way you’d need something else for that nothing exists yet.

TeoTwawki avatar Oct 15 '21 19:10 TeoTwawki

This seems like it should follow more of a fallback scenario. If Prepare, then use it, else default to other behavior

I thought it worked this way tbh. Anyways, this + if it doesn't random the numbers you put or something it should use one of the spells from the mobs spell list instead. So you could for example: Give mob Spell list: Quake, Stone4, Rasp Set function to make using stonega4 50% chance(math random 0.5) if it doesn't roll the 50%, it uses quake, stone4, or rasp at random.

ShiyoKozuki avatar Oct 20 '21 18:10 ShiyoKozuki

We should remove the column and pass an table with the skill list into this function, then the existing list can be consulted if needed. This should also extend to the mobskill version of this function

zach2good avatar Oct 20 '21 18:10 zach2good

this got mostly handled, I dunno if you want to retain the issue. I think the column still is there but isn't used now.

TeoTwawki avatar Aug 14 '24 02:08 TeoTwawki

Confirmed: Everything to do with hasSpellScript is removed/vestigial. Since there's a good amount of custom formatting in mob_pools, I'm not entirely sure how to remove the flag. I'd probably lean on a python script?

And then all of the hasSpellScript stuff in core can be ripped out, but the SQL queries have to be handled carefully - since they're in the middle of a sea of offsets. Possibly a good time to upgrade those queries to the db:: system

zach2good avatar Aug 14 '24 07:08 zach2good

Dbtool should be able to handle updating the sql files. Update your DB, then remove the column manually (from the DB and the sql file), then python3 dbtool.py dump mob_pools to update all the inserts.

cocosolos avatar Aug 14 '24 15:08 cocosolos

Ohh, I forgot you added that!

On Wed, Aug 14, 2024 at 4:11 PM Corey @.***> wrote:

Dbtool should be able to handle updating the sql files. Update your DB, then remove the column manually, then python3 dbtool.py dump mob_pools to update all the inserts.

— Reply to this email directly, view it on GitHub https://github.com/LandSandBoat/server/issues/768#issuecomment-2289078453, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKTJIOK7XKWESM2JCZ44G3ZRNXSNAVCNFSM6AAAAABMPMK7LSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOBZGA3TQNBVGM . You are receiving this because you were assigned.Message ID: @.***>

zach2good avatar Aug 14 '24 15:08 zach2good