(SAFETY COMMIT)
Largely breaking change.
* Interfaces in front of data types (and 'managers'), some interfaces
removed.
* Data and configuration fetching.
* Check activation checking (config flags, isEnabled, hasBypass).
* CheckType (activation checks, factories removed).
* Lots of collateral signature changes, including IPlayerData.
The (I)WorldDataManager stores per-world data (+ per world per check
type).
* Raw configurations.
* Typical flags: check activation, debug, lag adaption.
* Generic data, such as check configurations or per world check data.
The (I)PlayerDataManager stores per player data.
* Check Data.
* Typical flags: debug
* Exemption
* Check data (and config cache).
* Further mappings and later OfflinePlayerData.
* The registration interface will allow defining, how instances are
handled for registered types (factory, proxy, what on world change, what
on logout, global removal handler, per player removal handler).
(I)PlayerData is intended to be/become the central access point.
* External interface is IPlayerData now.
* Per player debug flags, exemptions.
* Fetching configuration and data: local cache, relaying fetching to
registered factories and proxy-registries/storage (e.g. fetching
configuration from per world storage).
Other fixes/changes:
(+) Extend the debug player command (set true/false, reset to world
default, arbitrary check types).
(+) PlayerData maintains a currentWorldIdentifier (to be used instead of
ChatData in future).
(+) The WorldConfigProvider getAll implementation returns a
LinkedHashSet now, avoiding duplicates.
(+) Move DefaultGenericInstanceRegistry to NCPCore.
(+) Thread-safety considerations for DefaultGenericInstanceRegistry.
(+) Don't log errors on hasBypass checking. TBD: Instead intercept
during listener methods (or even as a feature within the listener node:
e.g. @ThreadContext(primaryThread=true, skipOffContext=true,
cancelOffContext=true).
(+) Add fight.wrongturn permissions to plugin.yml.
(+) Missing GPLv3 headers.
Broken/Missing:
* WorldData inheritance from default: propagate all changes done
directly to the default config to children (all worlds that don't have
an explicit world_config.yml set) - possibly add an OverrideState or
similar, (NONE, FROM_DEFAULT, EXPLICIT) and don't override EXPLICIT if
coming from the default. Calling override on the default WorldData is
not to be confused with calling override for WorldDataManager (override
for all worlds as EXPLICIT).
* Organize overriding for special circumstances (version dependent
activation and the like). Might want to add registered override
handlers to be called on reload automatically.
* Store generic per check type per world data in the WorldDataManager,
such as configurations and per-world check data. TBD: Factories, cleanup
(!).
* Most efficient referencing (IWorldCheckTypeNode, IHandle<something>?).
* All the registry stuff (see PlayerData).
* Use interfaces for auto registry (and a flag within
RegistrationContext?) - world unload, world change, player join / leave.
* (Data expiration handling including transition to IOfflinePlayerData,
because now data is a little heavier.)
* Further details.
Issues with bunny hopping remain (...), but this might help with
elevators.
Use of velocity entries has been made more strict (directly use, remove
previously queued ones).
* Lift-off side conditions.
* Force stop gliding for some side conditions, to avoid freezing.
* Set maxheight to 128 for elytra and levitation too (mods/plugins/etc)
- better refine checks to catch stuff.
* Fix special flags not getting set with initializing ModelFlying from
config.
* Debug log exceeding the maxheight setting.
* Mostly harmless: Add interfaces and access methods, including
convenience methods.
* Don't store methods in Bridge1_9.
Issues left, not limited to:
* Boost not recognized on occasion.
* Gliding state kept when submerged in water and moving normally like
when not gliding, e.g. ascending (head in water / fully submerged).
* Elytra lift off not accepted: Gliding state set near the water
surface, but survivalfly check runs. Might be fixed already, though.
[BREAKING]
* Due to adding a method to CheckDataFactory.
* Might alter data removal (less is removed, might've overlooked
something).
For MOVING, FIGHT, COMBINED, data removal for sub checks now is
possible, allowing for more precise resetting via commands. Other check
groups may follow on request / randomly.
Data removal for check data is logged now (only CheckData, not
IRemoveData in general), in case the debug flag is set in the data.
Later, a more flexible method may get implemented, accepting a String
id, so not only check type related data, but also any type of special
data can be removed (e.g. moving.locationtrace would not be a check
type, but allow resetting the location trace of a player).
* The player is at the coordinates.
* The last received ACK for an outgoing teleport has been received on
packet level. This is experimental, to be confirmed to a) do something
b) not allow abuse.
* MovingListener.prepareSetBack makes more sense than the previous
ambiguous naming.
* On confirming a set back, don't update the setBack field, only set if
null.
The aim is to have a more consistent handling and naming for set back
stages.
Because Spigot changed to fire the teleport following an altered move
end point with TeleportCause.PLUGIN, we have to alter set back handling,
so we can ensure to keep TeleportCause.UNKNOWN for setting back players.
Instead of altering the move end point, the event is just cancelled, and
a teleport is scheduled (with a dedicated TickTask method). Uncancelled
moving events mean removing scheduled teleports.
[BLEEDING]
* Comparably simple change - more places and special cases may still be
uncovered.
[BREAKING]
* Plugins that may rely on the exact sequence of things within NCP, as
it used to be.
Random
* Change "set-back" to "set back" everywhere for simplicity, and to
obfuscate the actual code changes.
* Set backs are now going through MovingListener.onCancelledMove instead
of MovingListener.onMoveMonitorNotCancelled.
* Illegal move handling would still use event.setTo.
Lots of issues remain with elytra, with and without boost. Selection:
* maxheight will trigger with the rocket feature, naturally. Mendable by
increasing it via configuration
(checks.moving.creativelfly.model.elytra.vertical.maxheight). Not sure
we'll just increase the limit or alter how it's dealt with (e.g. also
for sf: lock to a max / high slope value, independently of the set-back
and world height, alter as necessary).
* All sorts of transitions, e.g. onto ground, into water.
* Loss of boost right after adding (not sure if already fixed).
* What with hover, actually?
* Is the flight duration infinite with power 127?
* Issues with ascending after descending, even without boost?
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
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 (!?).
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.
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.
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.
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).
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.)
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.
* 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.
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.
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).
Use check-specific debug methods for convenience.
Add to: Check, CheckListener, BaseAdapter.
Relay to: CheckUtils.
Side effects:
* Remove constructor: CheckListener().
asofold