search

andune / HomeSpawnPlus

Home/Spawn control plugin for Bukkit
GNU General Public License v3.0
13 stars 10 forks source link

Minor bug with MCPC #4

Closed Watchful1 closed 11 years ago

Watchful1 commented 11 years ago

I am using MCPC+(https://github.com/MinecraftPortCentral/MCPC-Plus) and with a large number of forge mods.

If a home if set inside a block added by a mod(a custom liquid for example) and safeTeleport is set to true, HomeSpawnPlus will throw this error when trying to teleport to that home. http://pastebin.com/vAVTpCW9

Turning off safeTeleport fixes the problem for obvious reasons.

andune commented 11 years ago

Interesting error report. While in the 2.0 branch I have laid the foundation for supporting non-Bukkit servers, I've not ever tested HSP with anything other than CraftBukkit, especially on the current 1.7.x releases, so your results may vary. I am glad to hear turning safeTeleport off resolves the issue.

I can't promise to look into the issue in any detail, at this time I only officially support CraftBukkit and even that I barely keep up with as I don't play or follow the MC/Bukkit community anymore and haven't for some months now; I do still try to keep my plugins up-to-date or help troubleshoot things as I can.

That all said, if you have a stack trace you can share of the issue, that might hint at the problem and perhaps I can fix it (might be related to blockIds > 255, for example). I don't have MCPC running to test with, but if you're willing to do some leg work to bring me logs and try out dev versions, I'll see what I can do to help out.

Also, if you're tied to the MCPC dev community at all and you know of any equivalent functionality to safeTeleport in MCPC, I'm happy to look at the docs and consider tying into an API. I was never happy with having to write safeTeleport, I only wrote it because nothing like it exists in Bukkit and I got tired of being stuck in walls in earlier versions of Bukkit. I'd much rather use a server provided safeTeleport mechanism than have to maintain my own; maybe MCPC has this capability?

Watchful1 commented 11 years ago

I believe I put the stack trace in that pastebin.

I am not an actual dev for anything, but I am fairly well connected. I do not recall any safeTeleport functions in any forge mods.

I would be happy to help test stuff if it would be useful.

andune commented 11 years ago

Oh, I totally missed the pastebin link. Thanks, I'll take a look at it and see if it's anything obvious I can fix.

andune commented 11 years ago

Ah, yes, completely obvious. Whatever mod you are using has block ids > 256. At least up to 1083 according to the stack trace. The safeTeleport algorithm implements an array of blockIds so it's very fast to lookup safe blocks as it iterates through nearby blocks looking for safe spots. That array is only as big as it needs to be (256 for Bukkit).

So it's very easy to fix, I just need to allocate an array up to the maximum blockIds that are possible. Any idea what that number is? I'd find it unlikely you have 1000+ unique blocks so I'd probably just pick 2048, but let me know if you know of blockIds > 2048.

Watchful1 commented 11 years ago

Block ID's in forge can go up to 4096.

Thanks for the fix.

andune commented 11 years ago

Also note this means no custom blocks will ever be considered 'safe', since the safeIds are hardcoded based on Bukkit values. So the custom blocks will always be considered unsafe. I'm not sure what type of blocks these are, but for example if you built a whole house out of them and did a /sethome inside, HSP would not teleport there since they aren't considered safe.

If this is a problem, I can consider some external API that would allow you to tweak safeIds, or maybe even just a config item for HSP so you can add to the list. Also you could always just build your own copy of HSP with your ids hard coded, they are in this file:

https://github.com/andune/HomeSpawnPlus/blob/master/src/main/java/org/morganm/homespawnplus/util/Teleport.java#L83

Watchful1 commented 11 years ago

Some mods do add blocks that are safe, such as the liquid one I was testing with, but I'm pretty sure they are nothing you would build a house out of. Regardless, each mod does things it's own way, so there is no reliable way to tell what is a safe block and what isn't. Additionally, the block ID's are configurable for mods, so it would be impossible to hard code them for a general case like it is with vanilla. As such, a config option might be helpful, but as safeteleport is far from an essential feature of this plugin, and this is very much of an edge case, it would be a very low priority.

Thanks for all the help.

andune commented 11 years ago

To that point, I don't know if I mentioned it already but you can always just turn safeTeleport off. Just set core.safeTeleport to false.

Or once this 4096 fix is released in a build or dev version you want to use, you can keep it on if you like the core behavior with the mentioned caveat about no safe custom blocks.

andune commented 11 years ago

FYI the latest builds on Jenkins have this change now, not sure how the CraftBukkit package names match up with what is in Forge, so not sure if that's usable for you or not.

Watchful1 commented 11 years ago

once again, thanks for your help.