* 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.
360 ticks = 6 seconds, according to the wiki, the max possible duration is proportional to dmg taken, so 7.5 seconds would be the max duration (aka 450 ticks)
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.
This renames some methods in the Utils class to better reflect what they
do or what they provide back. It should be clearer what these methods
are for now than before.
Some anti-cheat calls were changed to "You have been Bounced." I don't
think we need to tell clients why they were disconnected so they can
tune their hacks better.
See the changelog entry for this. Basically, it's worse to keep
confirmed broken noclip detection in, even if it detects noclip 90% of
the time, because a random hacker can make a better noclip system and
just evade it 100% of the time by exploiting holes.
Note: even in the previous iteration of the code the search function
returned -1 if it couldn't find it in the system, which would have made
removal of projectiles by ident and owner impossible because they
weren't found anyway.
Either way, this is related to fixing #1584.
This reimplements warnings that CheckTilePermissions previously had. It
defaults to on because every single call currently in TShock expects it
to be on.
This moves ice tile placement and processing from
TShock.CheckTilePermissions to TSPlayer in the form of the new boolean
'HasModifiedIceSuccessfully.' This is such a stupid thing we have to
track, but we have to track it.
Previously, we duplicated all of the check permission code and inserted
special ice code paths. This duplicated a ton of code for little gain.
The result of moving everything is that the control flow is easier to
follow.
In Terraria ice tiles are placed and melt on a timer so it's necessary
to track them being placed and removed to permit tile events that would
otherwise be blocked due to region checks and stuff. They're usually
fairly harmless blocks, and without this code, ice wouldn't work
properly. It's not ideal for this to be in TShock at all.
TShock.CheckTilePermission(ply, x, y, paint) has been replaced with
TSPlayer.HasPaintPermission and TSPlayer.HasBuildPermission
respectively. These two methods dramatically simplify the logic required
to process build permissions.
Still todo after this commit:
1. Implement warnings for when a player fails a permission check on
build. This is probably going to be accomplished by a default argument
to the HasBuildPermission signature.
2. Create a variant that checks for ice tile permission and use like
HasPaintPermission() so as to remove the other
TShock.CheckTilePermission method.
Annoyingly, all of the existing methods that start with Check seem to
"check" if they player _doesn't_ have permission (true = no permission),
which makes nearly every call negative.
This addresses feedback from @QuiCM and @ijwu, who pointed out that C#
allows you to specify the arguments that go into a call during
invocation, which dramatically improves readability.
TShock.CheckRangePermission is now TSPlayer.IsInRange, but the most
important thing is that this method returns the opposite of what the
original did, so all of the calls that would go to it are now inverted.
Bonus: Introduces a new GetDataHandledEventArgs for use in events that
have players, data, and need to be handled.
I was originally going to modify GetDataHandlerArgs, but that comes from
the EventArgs class of args that isn't handled, and changing that to
extend HandledEventArgs would imply we care or check to see if those are
handled and we don't so we're stuck with this implementation for a
teenie tiny bit.
This just changes IgnoreActionsForClearingTrashCan to meet the
same naming scheme for the rest of the old ignore checks. For
consistency. Consistency is nice.
This replaces IgnoreActionsForCheating in TSPlayer with a new
IsDisabledForStackDetection field that tracks the same basic data.
The previous way we did this was storing a string as the "reason"
why a player was disabled for cheating, but it only stored the last
hacked item stack that caused the check to fail. Since we already
have OnSecondUpdate which notifies on _all_ items, we don't need
to store this info in such a useless way anyway. They'll find out
in one second what they need to remove in a more alarmist way.
This is the first commit in a series to rewrite CheckIgnores()
into whatever its replacement becomes.
IgnoreActionsForInventory was probably used by the SSC system prior
to when we had in-game support for SSC (ergo, when we just checked
to make sure you had removed all items before joining and worked
our way up in inventory data to track it). I could be wrong about
this though.
Now, IsDisabledForSSC tracks only if a player is shut down due to
SSC, rather than a reason that gets broadcast.
