Later the ordinary envelope should be checked by survivalfly, possibly
adding exceptions for specific side conditions, e.g. elytra is worn.
Could lead to unifying cf + sf some day, rather using different kind of
sub-check methods, depending on side conditions (flying, allow flying,
elytra, ...).
Missing:
* More testing and adjusting.
* Other cases found, but not yet done. E.g. multiple 0-ydistchange.
* Jumping with oddd gain (2x 3.8 instead 0.42 + 0.33333).
* Lost ground and all the cases apply here (use only sf or refactor).
* Do or start to distinguish ascend vs. descend, if gravity is used, if
modifiers are used, similar, based on the model in use (the vertical
check part had never accounted for descending anyway). Shuffle chunks of
code.
* Reflect new options in the configuration, sort in more to vertical and
horizontal sections.
* Alter set-back handling, use ordinary set-back for maxheight,
prioritie other violations over maxheight.
* fly-nofly transition: Clear active horizontal velocity, allow less
horizontal friction.
* Increase horizontal speed to 420 for spectator mode.
* Add more tags with specific cases applying.
* Adjust logging format of hdist/vdist.
This probably isn't a usable state, more changes and testing will
follow.
If false positives with players receiving knockback from pvp hits
increase, setting checks.fight.pvp.knockbackvelocity to true instead of
default will force-activate the workaround.
Debug logging would reveal if this works or not. If the velocity events
fire correctly, the horizontal components will be clearly greater than
zero most of the time with pvp hits. [Can't test this right now.]
Very coarse modeling, players likely are able to abuse this and there
are verly likely more false positives.
Especially elytra will have issues:
* Players can fly very fast.
* Elytra will make players fly even faster than the set limit, resulting
in false positives at some point. Setting the allowed speed that high
will yield the problem of players being able to abuse even worse. Thus
limiting to the speed of spectator mode. Modeling will be changed to
accomodate for gain vs. max. distance and other.
This won't cover all effects of internal changes, some blocks might be
interpreted wrongly still, some shapes may have changed in an
incompatible way (e.g. skulls).
This seems to be more appropriate, for the case of multiple checks
triggering, but also for better performance, in case the check doesn't
even support input-specific penalties.
This is a first step in, which doesn't change the default behavior,
however it might break plugins that rely on certain internals.
PenaltyAction allows to do something with probabilities to consider,
including the possibility to select the first applicable penalty or
applying several penalties. There will be player-specific penalties,
which are applied during ViolationData.executeActions always, and there
will be input-specific penalties, e.g. for applying within the event
listener.
Potentially breaking:
* Return value of executeActions is now void for Action + ViolationData.
* Return value of Check.executeActions is ViolationData now.
* CancelAction is now extending PenaltyAction.
* CancelPenalty may cancel, but might not, due to probability.
* IViolationInfo.hasCancel -> deprecated, now returns willCancel().
* IViolationInfo.willCancel is now used, applicable penalties are
estimated on creation of ViolationData.
* Custom actions can no longer be used to cause cancel. Only penalties
can do so now (due to the return type change). CancelAction is still
there to keep a simple action for canceling.
Not yet:
* InputSpecificPenalty support for fight checks and using them in the
default actions.
* Configuration for penalties (currently only a plugin could override
the action factories, later penalties may have a probability to apply,
reference each other, allow first match, apply several at once).
Introduce an interface to indicate if a CheckDataFactory or IRemoveData
instance can do something better than removing all data, in case the
system time ran backwards. An extra method in data manager is used
instead of clearAllData, which will test for implementation of that
interface. Concerns CheckDataFactory instances accessible via CheckType
and IRemoveData instances registered with DataManager (via
NoCheatPlusAPI or directly).
Implementation details and related changes:
* TickTask: Let ActionFrequency handle time running backwards (spikes).
* NetData/Factory: Use HashMapLOW. Selectively clear/adjust.
* MovingData: Keep past move tracking (and some other).
Remove ambigue method, apply a different default margin, up to the next
block in steps of 0.25, if within 0.35 reach.
Default methods use this correction now, so some places might check with
too high a margin now.
Not entirely sure this will still protect from anything, shortish:
* Remove cancelling due to coarse pre-checks.
* Rather correct the end location back onto the end block somehow.
* Retry ray-tracing with the pitch and yaw of past flying packets.
* Let the direction check handle the off-too-far part and let people
blame that one for remaining amounts of false positives.
Missing:
* Should confine by distance to last move, perhaps.
Use check-specific debug methods for convenience.
Add to: Check, CheckListener, BaseAdapter.
Relay to: CheckUtils.
Side effects:
* Remove constructor: CheckListener().
This is half a guess on base of a request. Hooks can now check if there
are any log actions, or cast IViolationInfo to ViolationData and query
if anything would be logged to a certain stream.
Alterations pending (allow query for multiple streams or just get
streams/configs, what to have in the official API in IViolationInfo).
Untested, unused. Intentions are:
* Be able to count any use of workarounds.
* Confine workarounds to side conditions, such as 'use once until
conditions are reset' and/or 'only use once conditions are set'.
* Have per-player objects and (attached) global counters.
* (Might think of: disable workarounds by configuration.)
(@Samistine).
Slight deviations:
* Random formatting changes.
* Use a linked list for changedCommands at first.
* Use list.clear after iteration, instead of calling remove repeatedly.
* descendingIterator instead of ListIterator + (has+) previous.
* Add mightBeMultipleMoves to MoveData.
* Use a utility method to test for skip_paper, both on setting
vAllowedDistance for vdistrel and in vdistsb.
* Set the skip_paper tag during setting vAllowedDistance for vdistrel,
as that is always run.
* Cancel/alter on lowest.
* Override cancel on set-back on highest.
* Adjust data on monitor.
Partly simplify, e.g. by just cancelling the event (nothing should
happen) - can't recall if there has been a reason to setTo(setBack)
instead of event.setCancelled(true).
* Call both most of the time.
* A convenience measure to have last move ground set, at the cost of
setting it 'too often' (players might not get checked by survivalfly at
all).
* Wrap calls with one PlayerLocation instance, for efficiency.
Move location-dependent properties from MovingData and MoveData to
LocationData.
Rough list of related Changes:
* Represent from and to positions as LocationData inside of MoveData.
* Have flags for onGround, inLiquid and the like in LocationData.
* Change noFallAssumeGround to touchedGroundWorkaround within MoveData.
* Add touchedGround to MoveData (to|from|workaround).
* (Remove involved properties from MovingData and use MoveData.from/to
instead.)
* Use MoveData and LocationData flags instead of PlayerLocation methods
in more places.
* Adjust various special case pre-conditions, based on past move data.
Other changes made on the fly:
* Allow maximum of jump gain and step height for ground -> ground.
* Add envelopes for vDistAir after teleport/join/respawn.
* Add cases for vdistrel.
* Extend logging on teleport (add cause, log set-back too).
* Reorder/fix vdistsb workaround checking.
* Reorder teleport handling.
* Remove small-range workaround for teleport [uncertain effect].
Immediate future follow ups:
* Attempt to only accept PlayerLocation for various setPositions methods
in MovingData (ensure to set MoveData with extra properties +
simplify/cleanup (public) methods with MoveData/LocationData).
* Relate to past move tracking for more workarounds, either to confine
pre-conditions more (inLiquid instead of toWasReset~somehow), or just to
be able to track a false positive at all (thisMove + 2 past moves
needed).
* (Fixes, etc.)
Distant future follow-up:
* Somehow merge with PlayerLocation, e.g. using LocationData inside
PlayerLocation internally, which means changing raw types to Object
versions, just like it's done inside of PlayerLocation right now.
* Possibly PlayerLocation is transformed to static methods with
BlockCache and LocationData as input.
Expected trouble:
* New/old false positives, due to replacing the fromWasReset and
toWasReset by more distinct flags from past move tracking.
* A workaround may have prevented other false positives
unintentionally, e.g. had been intended for liquid, but the
to/fromWasReset flags previously did include ground/noFallAssumeGround,
thus the workaround will not cover that case anymore, after the change.
* Forgetting something like checking touchedGround and to/from.onGround
or similar as a replacement for xyWasReset.
* Mixing up thisMove and lastMove for touchedGround.
* Mixing up touchedGround and touchedGroundWorkaround in MoveData.
This is a first version without any safeguards and without any settings.
A player who has any meta data for 'nocheat.exempt' is regarded exempt
from all checks. Suggestion is a boolean value (true), but setting
anything will do for the moment. Later String values or List<String>
might be supported to allow some kind of generic categories for
exemption (to be discussed with skill plugin developers).
This is not meant for permanent marking, but rather for "extra" events
like area damage with entity.damage(amount, damagerPlayer). The plugin
setting the metadata should also remove the metadata. In order not to
remove all protection by NoCheatPlus, plugins should fire extra events
or cancel events on a higher priority level than EventPriority.LOWEST.
To be continued:
* Relating to the 'allowed base distance' for horizontal moving allows
to judge speed without taking friction or bunny-hopping into account.
* Make use in bunny hop (seems to make 2-high ceiling + sprint jump a
little better).
* Later more workarounds should be confined to a minimum, using
MoveData.
* Namely walkSpeed, downStream.
* SurvivalFly: Alter method signatures to use thisMove rather.
* MoveData: Only initialize the necessary minimum.
* Add MoveData.alwaysInvalidated, to indicate random future purpose.
Should only be possible to happen, if an actions entry doesn't use 'ncp
delay ...' for teleporting, or if a hook teleports the player (both
discouraged).
Does break use of MovingData for last coordinates and distances (not
officially exposed API).
Other changes:
* Position resetting on teleport events has been altered.
* Some blocks/methods are guarded by checking for lastMove.toIsValid.
* Possibly other.
Also:
* Rename the walkSpeed constant to WALK_SPEED (nuisance to mix up).
* Alter conditions slightly in some places by using
thisMove/lastMove.headObstructed.
Issues remaining:
* Moderate speed increase with yDist 0 and hDist like 0.35 -> 0.45.
* Transitions head blocked -> not, can trigger hspeed violations.
* Remove extra conditions for from.isHeadObstructed in vDistAir.
* Account for yDistance in more places. Use the maximum of the default
margin and yDistance.
* Add a tag for not setting low jump, due to the head being obstructed.
Fixes:
* Issues with 2-high ceiling with normal ground.
Issues remaining:
* Ice floor + 2-high ceiling.
* Jump/bunny with head blocked, moving to where the head is not blocked.
There is a small chance that other plugin cancel these events. Since the
server ignores event.setFrom for cancelled events and also won't fire a
teleport event, this remains a problematic case.
Represents the first "simplistic" approach to block change tracking,
only attempting to make vertical push/pull work.
It seems that we need to add on-ground checking accounting for piston
moves as well, otherwise anything with pistons retracting will lead to
survivalfly violations. Pistons extending and retracting may also
randomly move around players, including dragging them into the piston
block with the bounding box (not center of player).
In order to make on-ground work, we might need to check in another
place, possibly check where resetFrom an resetTo are set. Performance
questions might remain, there might also be a slight redesign necessary,
in order to run some sub-routines more side-effect free, to check
several branches, including after-failure checking.
* Below 1.7 allow ground-to-ground hop with moderate speed. Might be
there is more speed possible, shortly tested on 1.6.4.
* From 1.7.10 on, hitting the jump envelope or having the head
obstructed is demanded.
* The GROUND_HEIGHT flag indicates, that players are on ground (and can
walk on) from getGroundMinHeight on, once a block collides. Thus an
extra case for isPassableWorkaround is necessary.
* Set GROUND_HEIGHT for ENDER_PORTAL_FRAME, return the minimal height of
the ENDER_PORTAL_FRAME block for getGroundMinHeight. (Also add XZ100,
just to be sure.)
