The rate limiting error message used the term "tokens," which could be
easily misconstrued to refer to REST auth tokens, and not rate limit
leaky-bucket tokens. Since we don't expose the internals of the leaky
bucket to end-users, this error message is essentially just not good.
Without knowledge of a leaky bucket/GCRA, it really makes no sense.
Therefore, this changes the message to indicate that the "tokens" are
rate-limit tokens. It also adds a hint that there's a config setting
that can be changed to raise the limit, which further makes it more
understandable, and also provides a reasonable hint as to what setting
to change to alleviate this problem immediately. This makes it easier
for users to debug and less likely they have to read old docs/wait for help.
This allows server operators to more easily locate their world paths,
particularly on Linux and macOS, where it isn't very obvious. To
determine where the actively loaded world is, simply run `/worldinfo`.
The default guest group is critical and shouldn't be removed without
either TShock doing something like automatically recreating it if it
doesn't exist, or not having a huge problem if it doesn't exist.
I chose to take the easiest path, preventing users from removing it. In
theory the message gives enough context to imply "okay, go change the
group now."
This should be a relatively small edge case but I wanted to resolve it
while I was here.
@punchready reported that there was an issue with the /warp send command
checking for position != (0,0). I tried to figure out what was going on
here, and I came up dry. I think this is vestigial from some other
database or file format storage. When @MarioE converted everything to
the new warp system, he preserved the check. The problem is that the
check seems to be based on the idea that a non-existent warp would
return the default constructor (which would contain Point.Zero).
Instead, a warp not found returns a null now.
Therefore, the proper thing to do, as implied by @punchready, is to
simply nullcheck this value instead.
TShock.ConfigFile was deprecated and therefore changes applied in
previous commits were not applied until now.
See:
* f567486c47
* 39147355c1
* 597e403d50
This commit allows server operators to disable build permission failure
notices. This is because some servers wish to have a "silent" operation
mode where the server doesn't send out these messages to users. This
makes sense, e.g., when the server is a "museum" and isn't intended to
be changed.
The default value for the respawn timer was 5 seconds, but this always
looked wrong because the default game respawn time is at least 10
seconds for classic characters. This can always be changed in the config
file, but looks significantly better when it matches the default imho.
This commit fixes an issue where players could bypass the respawn timer
by using /home. Specifically, TShock rejects the command if the player
is dead.
Basically Color.Green looks ugly and Color.LightGreen looks less ugly. I
could see changing the default to something like pure green, but I think
this might cause people to be like "yo why is this so bright."
Now arguably this should be configurable but I'm not going to
revolutionize the world by abandoning far more important work to allow
more artful messages. I just think that Color.Green is ugly enough that
I can't tolerate it.
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).
Like all of the other server mods I've used use /slay and not /kill and
it's really frustrating to type the wrong thing so I'm just going to add
an alias and hope nobody else minds.
This partially implements ISO8601 + RFC3389 dates for backup filenames.
As noted in the changelog, this was a sponsor request, and also, it's
impossible to put the `:` literal in most files in most filesystems. So
as a result, this is technically not compliant with ISO8601 or RFC3389
but it is closer.
Also, wait, I don't even know because I don't have enough money to buy
ISO8601 so we'll just assume this is compliant. Job = done.
This is the first commit in a pattern that I'd like to follow. The
concept is that we specifically create handlers for things that are
"illegal per se." That is, there are no possible situations (in the
current protocol) where a packet of this type is received from a client.
In this case, I moved the emoji handler out of the Handler just for
emoji, since it seemed like an obvious case.
The rule of thumb is simple: if something is illegal per se, there
should be no possible way in the vanilla client to achieve this result.
If a player sends this combination of packets they *must* be hacking.
Not that there is a 99.9% chance they're hacking, but that there is a
100% unambiguous chance that they're hacking.
Something is illegal per se if it can only be created by a hacked
client. If there's a crashing bug that a normal player can do with a
complex series of vanilla events, that is not illegal per se.
The goal of this namespace and class of handlers is to handle exactly
one type of protocol violation, and remove the packet accordingly. If it
is ever reported that the packet can be sent from a vanilla client, the
check must be removed as it is no longer a per se violation of the
protocol.
This fixes false positive cheat detection when throwing rotten eggs at town NPCs while wearing Frost armor set. Also made the debug and kick messages more clear for future reference.
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.
This commit adds a fallback to address problems with FindByNameOrID
potentially returning ambiguous results. Now, in response to a multiple
match error, a player can specify tsi:[number] or tsn:[exact name] to
match a user ID or name exactly. This behaves analogous to the old
behavior of the search method.
Currently, the TSPlayer FindbyNameOrID method aborts if it finds an
"exact match" based on this criteria:
1. If the player ID is on the server, it must be the thing we're looking
for. Therefore, return that.
2. If the case sensitive "exact match" is on the server that isn't an
ID, that must be what we're looking for. Therefore, return that.
3. Just yolo and downcase everything and return any number of matching
players next.
This commit changes the behavior because some players have been joining
servers with ambiguous names, like `1`. In the current system, this
player is difficult to query because they're an "ID" and therefore an
exact match will be returned even if a player name exists that matches
the criteria.
This also alleviates the issue of a case exact match falling down the
same trap. It's ambiguous enough in all of these situations that an
admin should just be using a player ID instead.`
UsingBiomeTorches: Whether or not the player has the torchgod biometorches ability enabled
HappyFunTorchTime: Whether or not the player has fought the torchgod before (for logic that checks for torchgod spawning)
unlockedBiomeTorches: Whether or not the player has the torchgod biome torches ability unlocked
This fixes a ridiculous typo in GetDataHandlers where we were setting
the UsingBiomeTorches flag based on having unlocked biome torches,
rather than actually being used. Thanks to @Arthri for the tip!
