SignatureBeef / Open-Terraria-API

Open Terraria API - Mac, Linux & Windows
GNU General Public License v3.0
96 stars 37 forks source link

Some suggestions #43

Closed Moneylover3246 closed 2 years ago

Moneylover3246 commented 2 years ago

Overall otapi 3 works really well server side but there was a few inconsistencies compared to the original source

PlayerAnnounce hook Originally this hook just checked if the hook was handled and returned accordingly. Now it logs to the console that the player has left or joined regardless image

Netplay.Open/ClosePort() Not sure if this is otapi related but in v2 all the code in those methods were removed. OpenPort in v2 image

OpenPort now image

All of these differences are pretty subtle but they get in the way while trying to work with the server

SignatureBeef commented 2 years ago

PlayerAnnounce hook

ah yeh, ill flag this for improvement. a few IL modifications and the event args should be able to allow/cancel both chat and console individually.

Netplay.Open/ClosePort()

is this causing issues or crashes?

because this was intended, as it can now be achieved by using monomod runtime events, rather than otapi forcing it on them. however, if linux/osx is having issues otapi would need to prevent it running on those platforms as this code should only really work on windows iirc. i see the existing relogic code should try/catch this so it should work...if it does not please submit the error in another issue and as time permits i will review and fix if i can replicate it.

by default otapi (without mods) should function essentially the same as a vanilla server, so i see no reason to always disable this code yet.

Moneylover3246 commented 2 years ago

I only use Windows so i don't know for sure of the Open/ClosePort methods are issues there. I only noticed it because the method is extremely slow. As you said using the runtime events easily fix this

SignatureBeef commented 2 years ago

Forgot this existed, next time please link this issue to your next PR, as it will help describe what the issue is. Might be able to look at the player announce hook over the weekend though, plus i will aim to implement the Netplay hooks in the new TShock beta.

SignatureBeef commented 2 years ago

PlayerAnnounce hook

ah yeh, ill flag this for improvement. a few IL modifications and the event args should be able to allow/cancel both chat and console individually.

this is now implemented in 8204e2b3f9550aa2743f80dd87f4ca92f6408453 cc #49

id say tshock would need to control the new arg.Result property accordingly though.

Netplay.Open/ClosePort()

for this one, id say this is something that can go in a config option but i havent looked into hooking that yet, so might be easier to also be something tshock controls, either always off or in a opt-in config option; or better yet via a plugin/mod which may be able to implement a cross platform implementation.