The per-config-path notifications would keep showing up, even if you
removed the paths, then run 'ncp reload', then alter any of them and run
'ncp reload'.
To fix this, the configversion.created value is set to the current
build, if no config warnings are there - which is the same, as what the
notification suggests as an alternative to removing the paths and
running 'ncp reload'.
To do this, isConfigUpToDate had to be moved from Updates to
ConfigManager, which makes more sense anyway. In addition the 'created'
and saved 'values' are set to the biggest thing found, instead of the
prehistoric static value.
Further a negative 'created' value will not be overridden anymore,
allowing to silence the config notifications forever. Not necessarily
recommended for the general case, but it can be useful/necessary with
maintained blueprints, e.g. with administering multiple servers.
One of the next steps will be to remove the DefaultConfig.buildNumber in
favor of setting a build number for each and every path added. All
provided we don't run into nasty issues here.
Another follow up could be to create an extra registry/config log file
and write all the values there, and only print the first 5 in ingame
chat.
A NO_RISK flag has been added, to allow skipping workarounds such as
packet level ack for skipping a set-back teleport. This remains
so-and-so, because in this case the set-back still would stay 'to be
done' and moves setting off from elsewhere would get cancelled, leading
to re-scheduling it, still:
* There could remain potential with micro moves, intentionally getting
set back.
* There remains an uncertainty with the telported logic initially not
having been made for a set-back location getting kept 'to be done' over
multiple server side ticks potentially.
* Having the ability to turn off this rare (?) case, allows faster
reaction, if issues should arise.
The default settings can be referenced by their ids:
* default.legacy for pre-1.9.
* default.cautious for not taking risks (such as packet level workaround
disabled, otherwise it's currently post-1.9 handling, working but not
optimal on pre-1.9).
* default.modern for the latest thing (currently post-1.9).
The defaults have been adjusted, according to so far experience:
* default.cautious contains flags SCHEDULE and NO_RISK now.
* default.modern is the default now, and contains SCHEDULE (but not
NO_RISK). This is used if nothing is specified in the configuration.
Where it's known that it's the primary thread, that test should be
omitted.
A remaining problem is the Check class, where the convenience methods
all will lead to testing for Bukkit.isPrimaryThread(). This needs to be
done differently.
It'll be hard/impossible to work around, if we have to check permissions
and meta data. For permissions we could do some kind of bulk/context
dependent caching and updating policy and check via PlayerData, but meta
data needs the Bukkit API to state thread safety. Future design could
also do without knowledge of the thread, if permissions are cached and
exemption by meta data is turned off (or also cached, but this only
works if other plugins don't use it for temporary exemption), a lazy
approach could pass on an AlmostBoolean isPrimaryThread.
For now, at least some of the frequently run moving checks use the
optimized approach.
* HashMapLOW for thread-safe access to PlayerData instances.
* PlayerData removal now used the UUID. More changes pending for storing
the UUID for reference rather.
* Use bulk removal for expiration of entries (one time lock).
Can't do much better than being there already. Thinkable trouble could
be with high latency and multiple teleports to different locations in
quick succession, so that cancelling the teleport will lead to the
player violating survivalfly again in the future, which means longer
freezing/rubberbanding than if we teleport now. However, current
assumption is, that it's better not to keep teleporting players around.
Instead of maps for each individual purpose, and the rather expensive
TickListener adding and removing, player specific task will be done via
one PlayerTickListener that can be registered with the TickTask. Thus
PlayerData has the access methods requestUpdateInventory and
requestPlayerSetBack, and so on, later more. For the
DataManager.playerData map it'll be UUID first now.
Consequently some calls have been altered to prefer passing Player or
UUID for PlayerData getting.
Breaks: DataManager.getPlayerData(String, boolean) has been removed, new
methods added to do the same without boolean or with UUID passed extra.
Following changes may repeatedly/randomly break PlayerData and check
data access (unless you use CheckType.getDataFactory), this may not
follow directly, but more or less soon. Even Later, CheckType will get
broken too :), in favor of class instances with dynamic registration
ability.
Basic direction is to concentrate stuff in PlayerData, getting rid of
all the static data stores, but also making access to shared data
more efficient (e.g. store last world id + name and permission cache in
PlayerData). Access will be more thread safe (only for PlayerData,
permissions cache, likely for fetching check data too, however returned
objects may have their own contracts).
* 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.
We do need to fix behavior to move on, so the intention rather is to
react more flexibly towards debugging results, rather than having people
use random configurations early on. Still this does allow for fall-back
configuration, e.g. for live servers.
NOTES:
* Later we will make the configuration set at 'default' adjust to server
mod and version.
* Overriding checks.moving.setback.method with SET_TO can lead to
PlayerTeleportEvents with TeleportCause.PLUGIN on newer versions of
Spigot, which may conflict with other plugins assuming feature-based
teleportation (possibly resulting in /back locations getting overridden
wrongly). For legacy setups this will be similar to restoring the state
of build 1066 for most.
There could still be places where distinction of the used method is
necessary, which would mean bugs.
* Don't unset teleported, if event.getTo is the same position as the
teleported (set back) location.
* Prepare (with) comments.
(Main driver is to be able to adjust quickly without shifting code
to-fro legacy etc., while dealing with much differing side conditions
for server mod + version, client versions with multi protocol support,
and other like bungee or not bungee.)
* 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.
Both schedule a set back and update PlayerMoveEvent.getFrom() with the
set back location coordinates. This way, either the next incoming move
or a teleport event can confirm the set back location.
When a set back is scheduled:
* Cancel other teleports early. (x)
* Prevent Portal use. (x)
* Vehicle enter (not on vehicle set back). (x)
* Prevent attacking.
* Interact block. (x)
* Break block.
* Damage block. (x)
* Launch projectile.
* Place Block.
* Interact entity.
* Open inventory. (x)
The list is incomplete and adding/removing items remains subject to
discussion, having differing impact/severity for different actions. As
long as setting back rolls back to last ground, it might be better to
prevent some type of actions. Not all cancelling is logged.
(x) Probably most important for consistency, avoiding some types of
potential abuse.
A common framework
for "prevent types of action" during whatever-handling also is something
to consider.
Optimizations:
* Move handling some rare cases to methods (MovingListener,
PlayerTeleportEvent handling).
When the captain leaves the boat, the vehicle data of a passive player
passenger may be set now, so it won't set them back to where they
entered the vehicle.
Likely other places still need adjusting.
Tested with a pig. It's not nice.
* Vehicle envelope needs a lot of overhaul.
* Force fall set backs may be more nice to have for in-air downwards.
* The set back locations can be from seconds ago, with different
passengers than at the time of the set-back.
