* Fix scheduling vehicle set-backs: disabled by default.
* Adjust detecting already set set back in morepackets (vehicle).
* Always warn if a set-back gets overridden.
* Adjust debug logging and comments.
* Some data loss.
Looks like we're running into set-back loops, unless we can control this
otherwise. It's more safe and consistent for our context, however it
leads to nested events. Vehicle exit, player teleport and vehicle enter
will fire from within handling whichever event the vehicle checks got
called from, such as vehicle update, player move, vehicle move.
Possibly some justTeleported flag helps us here. Likely there is no
similar switch that we could use with scheduled set-backs. The proper
option would otherwise be, to use packet sending and/or even cancelling
packets selectively, which would more or less force us to hard depend on
ProtocolLib for supporting basic features of Minecraft, despite possibly
a
sensible move anyway.
* Implement VehicleMoveInfo and provide via AuxMoving.
* Use MovingData.resetVehiclePositions to reset past location on enter.
* Some auxiliary functionality.
* Route vehicle update and move through the same checkVehicleMove
method, to initialize things, making some decision about what locations
to use for from and to, and to ensure that firstPastMove is set.
* Adjustments and fixes (workaround for generics with PlayerLocation,
LocUtil.hashCode).
Prepare using VehicleUpdate and PlayerMove instead of VehicleMove for
vehicle moving. This change isn't intended to change
anything/much on the surface.
* Implement native IEntityAccessPositionAndLook for 1.9_R1 and 1.9_R2.
* Alter method visibility and parameters.
* Common pre-conditions.
* Route contents of both VehicleUpdateEvent and PlayerMoveEvent through
a common related method (also named onVehicleUpdate).
* Remove RichLivingEntityLocation, to be able to simplify more.
* Refine interfaces for locations (IGet... ISet... vs, I... for both).
* Implement location related interfaces in some places, related changes.
* Override hashCode for some of the location related classes. Use that
for storing location hashes instead of Location.hashCode. Auxiliary
methods for hashCode in LocUtil.
* Add onIce to LocationData.
* Renaming player vs. vehicles (likely incomplete).
* Possibly other related/random changes.
Line count is high for this change, despite not so complex. Next step is
to change VehicleChecks to use past move tracking to estimate from where
a vehicle is moving (left not compiling). Due to the lack of teleport
events, and due to entity last location being mostly useless, we have no
choice but to hard-set-back on anything that looks strange.
Roughly:
* Encapsulate past move tracking in a MoveTrace class.
* Have playerMoves and vehicleMoves (the latter unused).
* Resetting method for both player+vehicle including more packets each.
* Don't reset vehicle data on game mode change.
Not sure who started this, but apparently...
* NCP closes an open inventory, leading to an event for that.
* Due to the player having an item on the curser or similar, an item
drop event is fired.
* WorldGuard will kick the player due to a blacklist event.
* NCP will detect an open inventory and attempt to close it, resulting
in looping this.
Fix attempt (blind) stores the uuid of a player and skips further nested
closing of inventories.
Moving players and vehicles. Part evaluation, part preparations.
* Use more minimized types to demand for MoveData.
Likely future changes:
* Split MoveData into base MoveData extended by PlayerMoveData and
VehicleMoveData.
Might follow up:
* Might have a MoveDataStore providing the past move tracking in an
encapsulated way. To be used for players and vehicles.
* Attempt to have easy to share common auxiliary mechanics, so things
like 'mightBeMultipleMoves' and some of the associated resetting logic
can be a common routine for both players and vehicles.
* Similar.
Might follow up later:
* Don't laugh: consider lost ground to be made abstract enough to be
used for both players and vehicles.
* Track vehicles independently of players (tandem fly!).
Directly following:
* Boat fly check based on VehicleUpdateEvent and fetching last pos.
* Implement a native access based provider for
EntityAccessLastPositionAndLook, after testing the reflection based one.
Likely following:
* Implement the same fly checks based on PlayerMoveEvent for horses and
pigs too, for the case server-side fly checking is disabled.
* Configurability for individual types of enbtities, at least a flag for
activation.
* Not sure if a fall-back to VehicleMoveEvent should be kept, setup
shouldn't be all too complicated.
* Add a late in-air phase skip once workaround.
* Adjust oddSlope workaround ids to be lower-case and not contain wrpt.
* Add workaround counter for back to surface.
* Create interfaces for positions/locations. Use in RichXYLocatio,
TrigUtil.
* Add a basic implementation for isOnIce to RichBoundsLocation.
* Optimize prepare in RichBoundsLocation.
* [ONGOING] Elaborate on SetBackStorage.
Breaks access to Trigutil and RichBoundsLocation, thus PlayerLocation.
Recompiling should solve issues.
Major: Sketch vehicle envelope check.
* Renaming fields, methods, packages. Moving classes to other packages.
* Additions and refactoring for set-back handling and location tracking.
* Increase amount of debug logging.
* Adjustments to current vehicle set back handling.
* AuxMoving: call clear() on setMCAccess.
Minor: Adjust block change tracking implementation.
* Use a class instead of an id, in order to keep track of used entries.
* Allow reuse of an id, if the block still is intersecting.
* Improves situation for simple setup, issues remaining:
* Random UNKNOWN teleport by server potentially interfering.
* Distances > 1.0, possibly resulting from split move handling.
* On-ground estimation and passable.
* Blocks with gravity are worse (likely on-ground).
* More in-depth checking of constraints of implementation.
* Note that the block change tracker currently is disabled by default.
Naming:
* Possibly not final.
* survivalfly: Does not fit at all.
* Separate official checks + sections for speed and fly: nope.
* runfly: not running.
* speedfly: mix up with too fast flying.
* moving: too much redundancy with moving.vehicle.moving
* envelope: so and so, possibly better with tags to be set.
Content:
* Prepare a vehicle moving envelope check (just basic pre-coding
bookkeeping and refactoring).
* Split vehicle checks to another class. Needs refactoring.
* Add ability for simple generic instance registration by an interface.
* Add new sub check type MOVING_VEHICLE, with configuration section,
move existing stuff there (moving.vehicles.., morepacketsvehicle).
* Breaks at least the use of check type MOVING_MOREPACKETSVEHICLE.
FunFact:
* Try CheckType setup with MOVING_MOREPACKETS_VEHICLE(MOVING, ...) to
see why there is a test for this kind of thing.
Bugs:
* Old configuration paths don't seem to get removed with @Moved.
Missing:
* More refined vehicle checks.
No public method signature changes, but methods are moved to super
classes.
Likely not the final word. Think of RichLivingEntityLocation (horse head
bump, ...), and using more clean setup methods, as well as a better
implementation for isOnIce and similar.
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.
