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.
Only called in one method, the stack hack detection can move to
TSPlayer as it only ever operates on one player.
In a future commit, this will replace the stack hack detection
OnSecondUpdate() and also set the disabled flag if a player has
a hacked stack when called.
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.
Hooks have this fancy .Register method when they're of type
HandlerList<Args> that high made but we never bother to initialize
any of them, so .Register doesn't work because it's null.
This solves that problem by just initializing all handlers. Thus,
.Register works, and thus, you can register hooks with priorities.
This reverts commit 3f79a904da.
If prefix is < 1 and we block this event, clients can no longer
delete picked up items. This is what caused what Joshwoo reported.
http://rubyonrails.org/doctrine
The problem with most programmers is that they refuse to see any
logic in thinking about a problem from the other side, even just
once. You can't argue with someone over what opinionated "programmer
happiness" things are because logically you'll always be outmatched
with "well you can just do it this other way."
Take this example. How is !args.Player.HasProjectilePermission any
easier to understand than args.Player.LacksProjectilePermission?
-> One is direct: it focuses on what a player doesn't have.
-> The other is indirect: it's the inverse of have.
You can read one in a sentence and think "so if a player lacks a
permission then this happens" whereas the other is like "invert
if a player has a permission." In this soupy mess of a codebase
where you're trying to sort out what 300 magic numbers mean and
what everything else is trying to do, then it's kinda nice to
be able to read something and understand it immediately.
