Long ago in the early days of TShock someone asked why CTRL + C wasn't
handled and there was an explanation given along the lines of "something
something not supported on mono something something" or similar.
Attempts were made to try to handle console interrupts unsuccessfully
and the code was ripped out.
However, it's 2021, and we can now handle this signal and do the right
thing (which, ostensibly, is to save the world and shut down). Many
people like me reflexively hit CTRL + C because they want to shut down
the process. It's very infuriating that the current behavior results in
the server just dying and nothing being cleaned up properly.
Therefore, this commit changes the behavior to handle the interrupt,
save the world, and shut down nicely.
(If you still want to shutdown without saving the world, use off-nosave,
or idk, send SIGKILL).
TShock was originally designed to handle many things that Terraria did
not. Therefore, TShock always "took over" for the server password
prompt. We then added the ability to login via the password prompt if
you had an account, so that you could play on a server and login without
having to run /login in the chat window. Then, UUIDs were introduced,
and we added the ability to login via UUID.
This has created a cascading scenario where users are potentially
affected by many different things. We have always treated a user's
runtime intent as the most important: if a user sets something on the
console, it should be taken as the "most true" setting. In other words,
we believe that the most recent choice the user made is the valid one.
But for some of the config settings we have, we've made it opaque as to
how this decision making works. We also aren't clear what certain things
do by default.
Currently, if UUID login is enabled, a user will login "magically" and
bypass any password prompt. Even if this is disabled, though, users are,
by default, allowed to enter their passwords at the password prompt
instead of the server password. Both of these take priority over the
runtime setting.
The problem is that we haven't really made it clear if we should
override the runtime setting here. This is because the Terraria
interactive prompt asks for a server password, and one of the two
"bypass" settings is not a password setting at all. What do we respect?
I decided that the best approach is to just communicate really loudly
about these settings. If a runtime password is set, we'll warn users if
either of the bypass settings are "in play." If it's not set, we'll warn
users if the server password was set in config.json, just so they know
which password is being used.
If UUID logins are enabled we'll also warn users about that and the
security risks attached, no matter what. I don't know that we should
really have this feature, but we shouldn't get rid of it, imho.
The only thing I don't think we need to warn about is if login before
join is enabled. Login before join just acts as a way to speed up logins
for registered users. In an ideal world, users who shouldn't be able to
login should be banned. But I split the difference since we're warning
about UUID logins.
The only real downside to this change is that the PostInit hook gets
bigger. But dumping this stuff in another file/area/etc., seems dumb
since some of the logic exists here already. I think we can refactor
this later, but it's not my most pressing priority.
This whole change was inspired by the fact that @Onusai tried to lock
down their server but failed because of these settings enabled. We need
to be more transparent about logins, and this is a good first step.
- Previous Bouncer checks for GodMode (namely, Hurt) were removed.
- The command now uses the GodmodePower from core Terraria
- The toggle powers (which this command will now make use of) are now
reset on disconnect to prevent accidentally "gifting" godmode to an
unsuspecting player.
Can we store the type of the recent projectile as well? This could be used in my upcoming PR regarding the golf packet where I check for recently created projectiles, and their types. Having only the index is not useful in this scenario, because the projectile can already be killed, so cannot access the Main.projectile array to get the projectile type.
Can this be public instead of internal? Developers could make good use of it, instead of having to implement their own version of RecentProjectiles in their plugins.
Per packet debug logs are redundant for people with the packet monitor
plugin. If you need packet monitoring, please install the packet monitor
plugin.
Update the validation logic to be accurate:
* use pixels and not tiles
* allow master mode
* use npc position and not player position
Cleanup some style inconsistencies in NetHooks_SendData.
This reverts commit 7ad46abced. This
reintroduces the worldpath argument as per request from #1914, but at a
different name. This is because users have configurations like this,
which no longer work:
-world + -worldpath = crash
If you want to use -worldselectionpath to specify a world, you should be
able to use -worldname, but don't use -world unless you specify an
absolute path to a world.
No matter how we solve this we get a support headache (-worldpath +
-world = crash). This temporary stopgap should work to help address
issue #1914 until we can figure out a final solution. Since users are
impacted by this change, temporarily adding this back is the best move.
To be 100% clear, though:
-world + -worldselectpath without specifying an absolute path will
result in a crash that is unhelpful. Please don't do that.
These are high priority checks we really want to look at. I want to add
more of these debug statements to all checks in Bouncer and other parts
of GetDataHandlers, but I think this is good enough for now.
* Remove commented out warning disable
* Add initial ItemBans segregation infrastructure
* Add shell for initial OnSecondUpdate stuff
* Add comments yo
* Remove duplicated logic
* Split out more item ban code
This part of the fragments work is primarily aimed at reducing the
complexity of OnSecondUpdate in TShock and moving that check out into
the ItemBans subsytem.
Of major note in this is the removal of "check", which was a string
variable that tracked state and replacement of many of the item ban
activities with sane private methods that are at least somewhat
sensible. Obviously there's a lot to be desired in this system and I'm
really going for a run here by trying to continue a branch from so long
ago that I barely even remember the whole point of existence.
Still to do: GetDataHandlers related item ban code needs to be moved
into its own hook in the ItemBan system. Finally, there is a downside to
some of this: we're basically iterating over players again and again if
we keep this pattern up, which is kinda lame for complexity purposes.
* alt j: comment changes
* Move item ban check out of main playerupdate check
Separates out item ban logic from the rest of GetDataHandlers so that
item bans is more isolated in terms of what fragments is asking for.
* alt-j: convert indentation to tabs
* alt-j: fix botching source code
* Move item ban related chest checks out of gdh
* Remove chest item change detection from item bans
It doesn't do anything. If a user removes an item from a chest, it
bypasses this check. If a user adds an item to a chest, the server seems
to persist the change anyway, even if the event is handled. That's a bug
for sure, but fundamentally, it's not the item ban system's fault.
* Revert "Remove chest item change detection from item bans"
This reverts commit 758541ac5c4d4096df2db05ba2a398968113e1e4.
* Fix logic issues related to item ban handling
Re-implements chest item handling and correctly handles events and
returns after setting handled event state.
* Remove TSPlayer.HasProjectilePermission
In infinite wisdom, it turns out this is not a good method for TSPlayer
to have. It just checks the states of things as per what the item ban
system says is banned and then creates implicit relationships to the
projectile ban system.
Doing this effectively knocks down another external reference to the
item ban system outside of the context of the implementation for the
system itself and its related hooks.
This commit also adds context around what the heck is going on with some
of our more interesting checks as per discussions in Telegram with @Ijwu
and @QuiCM.
* Update changelog
* Remove useless ref to Projectile.SetDefaults
* Change item ban to ban based on ID not strings
I think I was so confused as to why we were passing strings everywhere
that I just felt inclined to continue the trend in previous commits.
From #1653:
>From latest version (2301)
If a player puts on a banned peice of armour, they get the message to say "You are wearing banned equipment"
After they take it off, the messages stop, but they keep getting disabled.
Solution:
Approx line 1131 in tShock.cs on `OnSecondUpdate` the `tsplayer.IsDisabledForBannedWearable` gets set to true, but it never gets set to false.
Change:
```csharp
if (check != "none")
player.IsDisabledForBannedWearable = true;
```
>To:
```csharp
if (check != "none")
player.IsDisabledForBannedWearable = true;
else
player.IsDisabledForBannedWearable = false;
```
Requires testing
The stat tracker has been offline for the last several weeks/months and
nobody has done anything to fix that. Because of that, GDPR, and the
fact that we haven't used it, we're discarding it.
If there's one thing I learned in this exercise, it's that letting
people add random python scripts to the repo like gpltext.py that
promise to be easier than find and replace is a bad idea.
