cheahjs / TerrariaAPI-Server

Fork is now over at https://github.com/NyxStudios/TerrariaAPI-Server
https://tshock.co
32 stars 24 forks source link

Add Npc position to NpcSpawnEventArgs #48

Closed QuiCM closed 9 years ago

QuiCM commented 9 years ago

Fixes #47

Ijwu commented 9 years ago

I don't agree with what Zack suggested because it doesn't fix the issue in a wholesome manner. If you provide the NPC position as separate properties of the event arguments then you are not really giving the NPC's current position.

You've also made it impossible for plugins to set the NPC's position as the position is overwritten later on anyway.

I would have followed my initial suggestion and simply moved the event invocation to after the NPC position is assigned.

GitFlip commented 9 years ago

@WhiteXZ Sorry, I hadn't noticed that you made a change and therefore I made my own change: https://github.com/Deathmax/TerrariaAPI-Server/pull/49

@Ijwu I agree with you, and you'll see my pull request does set the position above where we invoke the hook. I think this is the proper fix for this.

Olink commented 9 years ago

Commit 3ed336cb3fd50dbf796d9478e0a8d3c123ec92d9 should address this in a reasonable instance. Usage is the same, except in your code simple set handled to true if you are disabling the mob, otherwise set handled to false and let the code continue to run. This should allow the ability to filter spawns by location and other members such as wet etc.