Steps done:
* Add more velocity with less preconditions.
* Handle push with the player being just above the block where a slime
block is pushed to.
* Exception for vdistrel.
* Breaking:Move verVelUsed from MovingData to PlayerMoveData.
Gameplay issues remaining:
* Still too often fall damage is dealt.
* Friction envelope gets hidden with past-ground being set too often
(vdistsb).
* Potential for more edge cases.
Missing abuse prevention:
* Reinstate invalidation of past entries (currently turned off to
progress at all, will need another iteration).
* More confined preconditions.
* More/better invalidation conditions for velocity set for bounce
effects.
* Less fall damage.
* Flag all velocity added due to slime bouncing appropriately.
* Experimental concept for splitting up velocity, likely to be altered /
removed.
* Add a flag for (faked) pvp velocity.
NOTE: Invalidation of past entries has been deactivated to progress on detecting the stupid past bouncing at all. This will need another iteration.
Lots of issues remain, but vertical push/bounce with pistons is much
improved. Still there are show stoppers (false positives remain, as well
as occasional fall damage).
Use same/similar entry points, like static/classic bounce checking in
MovingListener. SurvivalFly still keeps one exception with the
after-failure Y_POS block move check.
Also check: https://github.com/NoCheatPlus/Issues/issues/5
There are typical cases to cover:
* Extra falling height.
* Fall damage where a slime block had been.
Thus adding a specialized method for bounce (just foot position) instead
of using the full bounds would be better, preferably just check within
the MovingListener (set bounce / adjust velocity there).
Still incomplete, could contain bugs (endless loops, perhaps).
Invalidation
mechanics may need to be refined.
Not covered:
* Exemption for moves resulting from horizontal push/pull.
*
* In case of non-full bounds + variable, allow ground to be found
underneath. Can concern fences (tested on 1.11).
* There has been a probably misplaced/leftover check assuming the block
above to be passable, without checking for it (iMaxY). Since the
access.getMaxBlockY() has already been checked, this part appears to be
not of use, thus iMaxY has been removed from the signature together with
this exception.
If this causes something, please provide debug logs / circumstances :).
This is unfinished, but gives an outlook on what we may be able to do
with this.
* Double doors are not covered (upper/lower half are).
* Interaction with levers / directly with doors is not covered.
* Only doors + gates (not sure what else there is).
* More fine grained configurability is missing (+ only register
listeners if needed).
* Possibly other things.
Tracking BlockRedstone seems to be more promising than BlockPhysics, as
long as we don't have to inspect neighbour blocks to check for door like
blocks at all.
Currently the oldest available entries are used, as if the player had
the maximum allowed latency always. Later on, at least attempting to hit
a global latency estimate (+window) should be attempted.
We'll still have a (moderate) fast-forward implementation to see what
opportunistic checking can do on live servers (elevators, doors etc.).
Intention is to work towards passing stored IBlockCacheNode instances to
methods in BlockProperties, i.e. to have
isOnGroundInAnOverlyOpportunisticWay implemented.
If you need one of the old BlockProperties signatures (or a more
simplified one), just open a GitHub issue.
This is a slightly peculiar change.
* Passing node and nodeAbove is a little bit odd, despite seemingly
efficient at present.
* Later we might need a BlockCache instance that busies about past
states (currently too complicated to implement).
* Uncertain impact (could perform better, could perform worse).
* Possibly left out a couple of places.
* Might have introduced bugs (fast forward).
On the other hand there may be a lot of other types of changes, if we
ever go for a less opportunistic implementation, which can not be
estimated simply, so this may be about the best that can be done, to get
a quick step in.
Already have the signature use IBlockCacheNode for node+nodeAbove, like
it will be done with other methods like collidesBlock.
This will allow calling this with past block states, once other methods
have been adapted as well.
Within BlockProperties BlockCache and IBlockCacheNode ("read only") get
passed, removing id/data/shape from arguments. If a property is not set
in the node, it'll be fetched from the block cache, but the node is not
updated from outside the BlockCache. For subsequent calls, the node
would be updated by the block cache, if it isn't a node stored by the
BlockChangeTracker from an earlier time, and similar.
This way, opportunistic passable checking can be implemented, by
switching to cached nodes instead of id/data/shape arguments with lazy
fetching from BlockCache.
The name IBlockCacheNode seems to be appropriate, since we'll pass it
alongside with a BlockCache anyway.
Cross-world teleport to the end issue: set back hard, if the player
moves to the position they set off from the last world, which seems to
be a bug with some server types/versions at present (!?).
Some packets arrive with a null world for a player, possibly sent by
plugins - thus attempt to use a stored world name.
This is just a hot fix attempt.
Attempt to enable workarounds for lily pads with servers that support
multiple client versions. Add to the configuration at
compatibility.blocks.overrideflags:
WATER_LILY: default+ign_passable+ground_height+height8_1
HEIGHT8_1 just means 1/8 height (0.125).
The ALLOW_LOWJUMP flag was intended to be used in case of
ground_height+height100 or the like leading to issues with
sprint-jumping due to the low jump detection, however other special
casing checks for bunny hopping let this still fail (less than before,
but still), thus this flag might get removed. Keeping it for now, to
provide some kind of toolkit.
Technically, tracking actual speed / base speed seems promising, done
here via using the liftOffEnvelope that is maintained anyway, adding a
counter and sum value. [Base speed is something like the allowed speed,
without accounting for specialties like bunny hopping.]
Limits are checked for moving in ground+air or water+air and are
currently hard-coded. This simplistic first-step-in may easily yield all
sorts of false positives and other random issues, handle with care.
Can't judge side effects, concerning other passable like tests telling
the head to be inside a block by the micro margin, while passable allows
such a move now.
Not sure if this really pays, if most people use protocol-support
plugins that limit packets anyway. There could be some future use, e.g.
generic rate limiting with configurable implementation.
Other:
* New methods for RawConfigFile+ConfigManager to check for
AlmostBooleanS.
* Add all net check permissions to plugin.yml.
* ICollideBlocks matches the interface better. If we ever need ICollide
for something common (e.g. set(ICollisionSetup) allows to fetch
information about start conditions and run generic tests !? not really
sure if such will happen...)
* Add ICollideRayVsAABB, to prepare other types of direction/visible
checking (enable more precision, but also configurable leniency).
This time the focus is on the utilities package.
Possibly used, but not really official API:
* Move block cache to a 'map' sub-package.
* Move RichBounds/RichEntity/Player-Location and TrigUtil to a location
sub-package.
Not really official API, likely not used:
* Move AttribUtil to compat, since it belongs there.
* Split off direction check methods to collision.CollisionUtil.
* Move static BlockCache methods to map.MapUtil.
* Move food related methods from CheckUtils to InventoryUtil.
* Move vehicle/passenger related methods from CheckUtils to
PassengerUtil.
Not breaking:
* Move IdUtil to commons.
registry.feature is rather meant for stuff that registers automatically
with adding as a component to the NoCheatPlus API.
Package organization isn't done in a very consistent way, this is just
taking a step into looking at how it would/could look like.
Likely that kind of separation can't be kept, so things will in the end
be organized by their main topic perhaps, and a registry implementation
will have to state what will be registered in what way, and possibly
have a method to allow checking what's possible to register.
Main objective is to get rid of too complex setMCAccess methods and to
be able to store handles rather permanently instead.
* Remove MCAccessHolder.
* Add/refine interfaces and implementations.
* Change constructors.
The idea is to change internals towards having a 'default'
DisableListener executing onDisable before the data removal of checks
and DataManager have been called, thus plugins or even checks might
still access all sorts data with disabling the plugin.
* Only load chunks, if there is no illegal coordinates.
* If there is no valid set-back on receiving illegal coordinates,
attempt to recover from past move / current (bug), or kick.
* Add a way to stay updated about the latest registration state for a
class.
* Add a class for the registry.
* Let the registry log all registrations.
* Make super interfaces for LogManager (simple logging of
String/Throwable).
Missing:
* More streams (REGISTRY, PLAYER/CHECK_STATUS/EVENTS or just CHECKS at
least). Make status rather the plugin status. Registry could have an
extra file.
* More efficient IGenericInstanceHandle use (wrap + reference counting).
Attempt to treat fake players less Concept is subject to change, might
want fall-back methods or skipping native access in general where it's
not needed (thus not need to check for native entities).
Other
* Don't insert dataMan into disableListeners twice.
This is neither complete nor final. Intentions are to group interfaces
better, rather organizing packages in a flat way.
At some point there will be other major move-arounds, but that'll
hopefully be a point where we have a better idea of where to put what
(...). For now the approach is to move interfaces/things rather where
it's not interfering with profane exemption API use, preferably neither
taking down the top level API layer
(NoCheatPlusAPI).
Added deprecated interfaces to prevent cncp to break too quickly.
Outlook:
* Classes that are rather only expected to be used internally for setup
will likely get moved around freely.
* Classes that have been added since last release might also get moved
around freely.
Configurable. Falling blocks, piston, end portal, roughly. Destructive:
the entity is removed.
(In addition some of the feature tags are added regardless activation
flags, because 'ncp reload' could change things anyway.)
Feel free to suggest alternatives/variations...
Except with join/respawn, the methods will not load chunks if close by
past moves have extra properties set, assuming the chunks are already
loaded then.
Change to 'ignoreInitiallyColliding' and only ignore the first block, if
it really is colliding right now. The flag is not resetting with
set(...) anymore.
* Make RayTracing and PassableRayTracing implement interfaces. (rough,
not used in tests yet etc.).
* Optimize ray tracing use in BlockProperties.
* Add an axis-wise implementation (buggy, hardly tested, thus not used
yet).
MCAccess will be split into smaller providers with time, so do expect
more breaking changes of this type. If this is an issue, please contact
us, so we can see how to smoothen things. E.g. we could still make
MCAccess an aggregate, that just delegates to the more fine grained
providers, or we could provide other
(default) aggregates.
This also adds a Bukkit-based provider for future updates.
* Call removeInvalid if many elements are queued on add (cheat not
sending moving events, player takes damage), and on clear, in order to
allow detection of unused velocity with not sending moves.
* Add get+set for unusedActive.
Simplistic unused vertical velocity tracking for starters. Only
activates with debugging set.
Needs on-the-fly debugging (all) or at least debug set for moving
(config: checks.moving.debug: true), and needs debug set for fight to
check on damage/attack (config: checks.fight.debug).
Method is simple, roughly: Keep track when the player has last been on
ground, or when their head had been blocked. Based on that, we can
attempt to judge if invalidated velocity entries might be cheating.
There is more aspects to cover, and this is not a check, it just will
debug information to the log file. Would appreciate feedback on if/what
this will log with noknockback cheats on :) (false positives are most
welcome as well).
* Account for off hand in more places.
* Use bridge methods to get rid of warnings for now.
* Adds utility methods to CheckUtils.
* Do not allow left click on off hand (knockback).
Since merging by distance only creates false positives (one would have
to increase the bounding box for an entry, but the angles would never be
100% right), we never merge, instead we don't add a new entry if the
position is the same, the time value is not updated in this case. For
validity of an entry you always have to consider the time span until the
previous (younger) entry or until now for the latest entry.
Rough changes:
* Use an interface for accessing trace entries.
* Use a linked structure for the actual trace.
* Use maximum age and size to limit the number of stored entries.
* Use a pool to somewhat limit object creation (size may need
configuration or scaling with number of players).
* Since the trace starts empty, have the field be final.
* Keep trace elements if settings are changed, cut size if necessary.
* Remove obsolete tests.
Potentially missing:
* Usage of LocationTrace has not been checked if we need to account for
the time of the latest entry not necessarily being updated (!).
* New tests, e.g. accounting for the expiration of entries.
Follow ups:
* (Extend fight/loop checks to a latency window mechanism.)
Upcoming changes will roughly be:
* Change implementation to a double linked structure.
* Implement/use something like ListIterator.
* Never merge entries, instead use some pool and time/extre-n as limits.
* A basic latency window implementation just for the LocationTrace for
preliminary experiments. [Track hit/miss all time + recent so and so
seconds, some extra cancelling/invalidation mechanics, allow to test
complement 0->window start and possibly window-end-> max latency for
some cases, cancelling mechanics may contain a buffer or a mixture of a
buffer relating to average miss rate]
Wild card exempt NPCs by default, encapsulate meta data checks here (if
exempted, if is NPC).
With these default settings Citizens NPCs should be exempted by default,
also all custom Player implementations that extend the Bukkit interface
NPC (not sure if such still exists).
Follow ups:
* Configurability.
* Exemption tickets to prevent others overriding your exemptions
(later).
Kept configurable to enable increasing difficulty for fly plugins use,
e.g. to allow players to fly within certain regions, but still deal fall
damage on falling.
Section at: checks.moving.vehicle.envelope.hdistcap
Default is 4 (extreme move fall-back) at 'default'.
Other entries by bukkit entity type name, if desired.
This allows forcing set backs for testing in a convenient way, e.g. with
setting the value to 0.3 for boat/default.
Main objective was to not reset the set-backs on setting back a vehicle
with a player, plus data resetting cleanup.
Specific changes:
* Prefer an existing set-back, in case the default one has been
invalidated.
* Do not cancel a vehicle set back task, in case no set-back is present.
* Avoid multiple resetting and redundant calls for resetting positions.
* Ensure the location set back to is set as a set-back location after
set back.
* Skip changing vehicle data with player moving resetting (e.g. player
teleport, player morepackets disabled).
Review and possibly correct/alter use of:
* MovingData.vehicleMoves.invalidate
* MovingData.vehicleSetBacks
* MovingData.clearMostMovingData
* AuxMoving.resetXYPositions
* MovingData.clearVehicleData
* MovingData.clearVehicleMorePacketsData
* MovingData.clearAllMorePacketsData
* MovingData|AuxMoving.resetVehiclePositions
* 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, ...).
