Skip to content
This repository has been archived by the owner on Sep 6, 2023. It is now read-only.

NPC Spawn Event not returning position. #47

Closed
GitFlip opened this issue Oct 28, 2014 · 15 comments
Closed

NPC Spawn Event not returning position. #47

GitFlip opened this issue Oct 28, 2014 · 15 comments

Comments

@GitFlip
Copy link

GitFlip commented Oct 28, 2014

I am making a TShock plugin and I am using the NPCSpawn hook. I can correctly get the NPC, but it's position (x,y) is always (0,0). Is this a bug or intended?

https://github.com/Deathmax/TerrariaAPI-Server/blob/d446a8288a5d7984f4298a507a64249865259472/TerrariaApi.Server/HookManager.cs#L632

public override void Initialize()
{
.....
ServerApi.Hooks.NpcSpawn.Register(this, OnNpcSpawn, 5);
....
}
....
public void OnNpcSpawn(NpcSpawnEventArgs args)
{
NPC npc= args.Npc;
Log.ConsoleInfo("NPC " + npc.type + " Spawned at " + npc.position.X + ", " + npc.position.Y);
}

NPC 61 Spawned at 0, 0
NPC 69 Spawned at 0, 0

@Ijwu
Copy link

Ijwu commented Oct 28, 2014

You're being passed a reference to the NPC object itself. If it reports a position of (0,0) it's most likely not due to TS-API.

Are you sure the NPCs are not being affected by another plugin? For example, I believe DieMob regions cause NPCs to appear at (0,0) then die or something like that.

@GitFlip
Copy link
Author

GitFlip commented Oct 28, 2014

Right I agree with what you are saying which is why I got very confused when the console was saying they are spawning at (0,0). I should also say they are definitely not spawning at (0,0) in game. I confirmed that the mob type was correctly spawning around me in game and not at (0,0).

The only plugin I have installed is the one I am working on which is not manipulating the NPC spawning ( yet because I cannot get their (x,y) ). The DieMob regions doesn't do anything to the NPCs that spawn. It checks if they are within a certain region on the GameUpdate Hook and kills them if they are in a region: https://github.com/InanZen/DieMob/blob/master/DieMob.cs#L166

Is there any chance you can make a quick tshock plugin where you simply use the NPCSpawn Hook for yourself and print out the same output as me to verify that it is on my end and not something with TS-API?

Thank you

@Olink
Copy link
Collaborator

Olink commented Oct 28, 2014

Why not just look at the source code and determine the issue??? Its open source for a reason.

@hakusaro
Copy link
Collaborator

+1 to Zack

On Tue, Oct 28, 2014, 11:55 AM Zack notifications@github.com wrote:

Why not just look at the source code and determine the issue??? Its open
source for a reason.


Reply to this email directly or view it on GitHub
#47 (comment)
.

@GitFlip
Copy link
Author

GitFlip commented Oct 28, 2014

Not sure it's a +1 really... I am trying to determine if this is an issue with TS-API or my plugin. I have looked at the source code a little bit, and actually do not see a problem because it is just like Ijwu said.

Can't we just determine first if it's me using the API incorrectly or if it's the API before spending any more time looking at code? You know that's usually how you solve bugs - you reproduce them. Anyone of you willing to confirm that it's not a bug in TS-API and I'm just wrongly using the hook? After we get through that I am 100% willing to help fix the bug since I kind of need this feature to work in my plugin, but like I said I would like confirmation that a bug even exists.

@Ijwu
Copy link

Ijwu commented Oct 28, 2014

I'll test it out as soon as I have a chance. For now I'm off to class.

@hakusaro
Copy link
Collaborator

@GitFlip well last I heard, Zack doesn't really have all the time in the world to check things like this right now, and I'm also at uni doing lots of stuff, so really, in this specific instance, what with the project being open source, self support is probably the best bet.

@Ijwu thanks for helping test this.

@GitFlip
Copy link
Author

GitFlip commented Oct 28, 2014

@nicatronTg I completely understand. I just graduated from Uni a couple years ago and now I work full time as a software engineer. In my very little free time I am working on making a Terraria MMORPG Style mod by releasing my own custom client and using tShock for the server modding. I've already decided I will spend some time this week looking into this more which obviously includes looking at the TS-API source code since, well, I need this functionality.

@Ijwu I appreciate getting the extra testing. I know it can be a pain to set things up, but it is really simple to reproduce if there is a bug, again I great appreciate the confirmation from someone besides me.

With all of this said I've been very happy with tShock and TS-API, so thank you.

@QuiCM
Copy link

QuiCM commented Oct 29, 2014

@GitFlip @Ijwu Some brief searching in Npc.cs shows that NPC position is assigned after the api hook is invoked.
http://puu.sh/cupcl/baeb1f3c66.png

Hope that helps in some way

@GitFlip
Copy link
Author

GitFlip commented Oct 29, 2014

@WhiTexz Thank you, you are exactly right. This would result in x and y to always be zero. I've never contributed to an open source project on github, but I am more than willing to make the code change to move the hook after we set the position. Does anyone else think that we should move it somewhere else, like right before we return?

@Ijwu
Copy link

Ijwu commented Oct 29, 2014

Sounds good. Thanks @WhiTexz.

@GitFlip If you know how to use Git, fork the repo in GitHub then clone the subsequent fork to your machine. Make your changes and update your fork using Git. Then you use GitHub to make a pull request.

Simply move the hook invocation to after the position is set, but not after active is set. This should let the hooks retain their ability to properly deny NPC spawning should they need to.

P.S. Come by the TShock forum and post about your mod in the off-topic area. I'm sure some of us would love a good discussion.

@GitFlip
Copy link
Author

GitFlip commented Oct 29, 2014

@Ijwu Thank you for the explanation, I've mostly only used github for my own projects, and never actually forked, or did a pull request to another project. I'll try to make the change tonight or tomorrow.

I'll also get around to posting in the off-topic area like you suggested, because I'd love to discuss it with you all. In the meantime feel free to view my post on the new official Terraria Forums: http://forums.terraria.org/index.php?threads/super-terraria-world-mmorpg-style-mod-server-and-client-mod.936/

@Olink
Copy link
Collaborator

Olink commented Oct 29, 2014

Fwiw my reply to look yourself was due to the following things you are likely unaware of.
1: This project is in maintenance mode and has been for a long time. We have no obligation to answer any questions and are very welcome to suggest you dig into the source code.
2: I know how lazy and useless Ijwu and White are, had I not said something relatively aggressive they would never seriously answer your question. My answer not only was correct, but it got you the answer you ultimately wanted.
3: Assuming you want to do anything remotely fun in Terraria you will need to get used to looking at the source code, for better or worse. This project is not for the faint of heart, and if my reply stopped someone unknowledgeable from constantly asking for help I am willing to suffer someone knowledgeable being put off. Unfortunately this is the type of community terraria brings and its a pita dealing with 12 year olds all day, something you don't get by being an adult and working in the industry.

Hopefully you understand I wasn't being cross with you, and you can understand where I am coming from. You are more likely to get help on the forums for this type of question anyways, hahaha.

Honestly without digging into the code i wouldn't know why the hook is where it is, so my suggestion is to just pass the x and y to the event and not change the placement of the hook imo.

E: nicatronTg; typos

@GitFlip
Copy link
Author

GitFlip commented Nov 1, 2014

@Olink Thank you for the explanation, I do understand where you are coming from - at least to some degree.

@WhiTexz I hadn't noticed that you made the fix that Olink had suggested. I went ahead and made what I think is actually the correct fix - moving the setting of the positions to right before we invoke the hook. You can view my pull request here: #49

@Olink
Copy link
Collaborator

Olink commented Feb 27, 2015

3ed336c solves this issue

@Olink Olink closed this as completed Feb 27, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants