Add Platform modification

[mistzzt]

public static readonly Platform Current = new WindowsPlatform();
// This line prevents OTAPI from initializing Console input encoding properly on Linux/macOS

This modification will make OTAPI initialized with right platform rather than Windows platform.

This should fix different world path (from vanilla server) OTAPI used on macOS or Linux.
In addition, the console input will be also fixed, which means we don't need any console input modification.

[hakusaro]

Okay, so my only problem with this is that it will change where world paths are initialized from on Linux and on macOS. This means that existing server owners could potentially have scary "data-loss" moments where they start the server, and it correctly picks the platform specific location for storing world data instead of the place where it's always stored data -- the mono equivalent of the windows path.

In the long run, this makes sense to do as it solves the console encoding issue without requiring a modification. In the short term, this could really scare some users into thinking that TShock has ran off with their worlds.

I'm not entirely sure how I stand on this, so I'm not going to review it either way. I think it's worth discussing the implications of this change before merging it, though.

[Ijwu]

It is worth discussing the implications, but if it means we end up better off then I think we can shoulder the burden of soothing spooked server owners. We can try to release as much documentation around this as possible as well, but we all know someone won't read and will head over to our chatrooms about it. It may just be something we'll have to deal with.

[mistzzt]

It is confusing when a starter on *nix finding their world created in Terraria or vanilla server disappeared.
For existing server owners, they should be informed this change like they were done about out-dated plugins.

[hakusaro]

Yeah -- that's also a good point. I suppose if there's any time to make this change, it would be now, during an update cycle, than midway through it. We can just put a big notice on the forums and add a chat command to maka linking to the help page so when people get scared they'll know why.

It's a really good point @mistzzt makes -- that vanilla doesn't do this. We haven't ever had a vanilla server on *nix before, so obviously now we're in the wrong.

[hakusaro]

Tested working on macOS. Loads the macOS defined platform worlds successfully.

[Ijwu]

Class is not renamed to match modification name. Not sure if this is intentional.

[mistzzt]

forget to rename it
I'll update it now
lol

[hakusaro]

lol

[Ijwu]

I'm approving this review, but I would generally prefer if @DeathCradle or @tylerjwatson looked at this. I'd hold out for either of their approvals as they are the mad scientists.

[hakusaro]

[tylerjwatson]

Okay why does this patch for a platform modification include file deletes from another modification?

[Ijwu]

@tylerjwatson the only deletes I see are in the UnicodeInputModification, which this modification is meant to replace.

[mistzzt]

@tylerjwatson By changing the platform, the vanilla server's InitializeConsoleOutput() will work properly.

As a result, we don't need UnicodeInputModification any more.

[SignatureBeef]

Regarding the IL of this modification, I'm thinking it might be best if a check was added to make sure that the static constructor only contains what you currently expect it to.
If relogic add more code to this in a future update I would prefer to know it broke rather than clearing the IL and loosing whatever was added.

[SignatureBeef]

Thanks for adding in the checks.
If the platform detection works like @hakusaro says it does then great, lgtm.

[hakusaro]

It definitely worked when tested on macOS (went to the library like it should for worlds).

…

On Sat, Apr 29, 2017 at 9:42 PM Luke ***@***.***> wrote: ***@***.**** approved this pull request. Thanks for adding in the checks. If the platform detection works like @hakusaro <https://github.com/hakusaro> says it does then great, lgtm. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#134 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAggpzzx0Itx8BaojbRubJ19yunT1vzCks5r1AM-gaJpZM4NJyJL> .

[Mark90]

@hakusaro pointed me to here, because I've noticed that on my server the 1585 travis build shows chinese characters on the server's CLI, like so:

Terraria Server v1.3.5.3

Listening on port 7777
Type 'help' for a list of commands.

: 㙛㌻罒罿睦晥㈱ㄳ㌲

With build 1583 there is no such issue, this is reproducible when I switch back and forth between 1583 and 1585. Details about my setup:

mmoes@markian:~$ locale
LANG=en_US.UTF-8
LANGUAGE=en_US:en
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC="en_US.UTF-8"
LC_TIME="en_US.UTF-8"
LC_COLLATE="en_US.UTF-8"
LC_MONETARY="en_US.UTF-8"
LC_MESSAGES="en_US.UTF-8"
LC_PAPER="en_US.UTF-8"
LC_NAME="en_US.UTF-8"
LC_ADDRESS="en_US.UTF-8"
LC_TELEPHONE="en_US.UTF-8"
LC_MEASUREMENT="en_US.UTF-8"
LC_IDENTIFICATION="en_US.UTF-8"
LC_ALL=
mmoes@markian:~$ uname -a
Linux markian 3.2.0-4-amd64 #1 SMP Debian 3.2.81-2 x86_64 GNU/Linux
mmoes@markian:~$ mono --version
Mono JIT compiler version 4.6.2 (Debian 4.6.2.7+dfsg-1)
Copyright (C) 2002-2014 Novell, Inc, Xamarin Inc and Contributors. www.mono-project.com
        TLS:           __thread
        SIGSEGV:       altstack
        Notifications: epoll
        Architecture:  amd64
        Disabled:      none
        Misc:          softdebug
        LLVM:          supported, not enabled.
        GC:            sgen
mmoes@markian:~$

[hakusaro]

@Mark90 can you test this? https://travis.tshock.co/t-1.3.5/1587/

[Mark90]

@hakusaro That's english for me!

[hakusaro]

@Mark90 so, all good?

[Mark90]

Tested 1591 as well just to be sure, and verified the ingame translations. All seems good :-)

[hakusaro]

Glad to hear it! Thanks for your help!