Paper
Paper copied to clipboard
Add Configuration for vertical Despawn Ranges
resolves #10177 config naming/structure might need some changes, i didn't wanna break existing configs so i just did it like this.
currently doesn't work for Raiders as those have extra checking logic in the removeWhenFarAway method, so i am not quite sure what to do about those
EDIT: also wanted to add that i am not sure if the normal despawn range should still impact the y axis. since your x axis is going to be higher then your y axis 99 % of the time anyways i will just have it keep affecting that aswell
I'm not sold on the format for the config with this change. There are several options that I can think of that I'd prefer
despawn-ranges:
monster:
hard:
xz: 30 # or "horizontal"
y: 5 # or "vertical"
soft: 20 # just putting a numerical value would mean it applies to both (as it does now)
or
despawn-ranges:
monster:
hard: 30
soft: 20
soft-y: 5 # basically the `-y` value wouldn't even be in the config if it wasn't being used
is there an existing way i can add optional configurations like that?
As you already said, this does not currently "work", as the distanceSqrt still takes into account the y diff. Given we already have changes to basically every line in this section, I think it would be fair to just completely rewrite this logic in a new paper block without consideration of keeping old code around/minimizing diff (given that this isn't minimal diff either, every line is touched beyond the two discard calls).
This would also heavily improve the variable names, making this whole mess a lot easier to read.
alright i will probably be working on the custom serializer and the rewrite tomorrow
Implemented the requested changes
bump
Is there a chance to merge before 1.21 arrives
if someone could comment on whether or not the config layout is ok that would be great, then i can quickly writeup a docs pr so that's also ready.
new config layout (as proposed by machine):
despawn-ranges:
monster:
hard:
horizontal: 30
vertical: 5
soft: 20 # just putting a numerical value would mean it applies to both (as it does now)
if someone could comment on whether or not the config layout is ok that would be great, then i can quickly writeup a docs pr so that's also ready.
new config layout (as proposed by machine):
despawn-ranges: monster: hard: horizontal: 30 vertical: 5 soft: 20 # just putting a numerical value would mean it applies to both (as it does now)
I think this config format layout is great Clear and straightforward
This PR can perfectly solve the problem I'm currently encountering. https://github.com/PaperMC/Paper/discussions/10520
Sorry, another bump, the stable version 1.21 has been released for some time now, can this feature be merged now? I really need it.
Brought this up internally, but I am less and less convinced that this "dynamic" syntax is that nice for us. Configs would now potentially change outside of a version migration because someone defined
hard:
xy: 10
y: 10
Would obviously be interested in other peoples input here, but a single migration and simply always using the more verbose layout would imo work just as fine.
Would configs changing node structure have any kind of impact though? configurate still only sees that Map<MobType, DespawnRange> and all (de)serialization regarding the underlying nodes is done by the serializer. Do you have any example of how this might have an bad impact?
I mean, mostly that the file changes without a migration notice in the _version field.
Now that I look at it, we'd also instead want to make it use default
instead of the current versions magic values.
And current layout brings a couple of annoying side effects.
When you only define hard: default
, the config makes sense. All ranges for hard despawning are defaulted.
If you insert
hard:
horizontal: 10
then vertical is implicitly set to default, even without it being present in the config. So now we are introducing a) paper changing the config file without the need to do so, potentially breaking/messing with server state managers (at least mine lol) b) it computes an implicit value.
Beyond the fact that we need to change this to using "default" values, need to drop the ConfigSerializable
annotation and also need to register the deserializer in the first place, I think it may be worth to (at least) look into preserving the config structure if people defined the same value for horizontal/vertical.
I can definitely change it so it doesn't doesn't create a node with a default value if it hasn't been defined in the config and change it so it uses default instead of the normal numbers. _version is there to make sure configs aren't loaded on incompatible versions (which i just realized i also have to up considering old servers won't play nice with this) not to track config history. I will just wait on this pr for some time and allow people to give their thoughts on it.
Yea probably best, I'll through it at my list to discuss with machine whenever we next sit down. But yea, if you could register the serialiszer and yank the config seria annotation, that would be great. Just so that no one forgets that xD
Already did that yesterday, see the latest commit. (not even sure whether i forgot to re add that on the 1.21 or 1.21.1 update lol)
Ah sweet, I must have pulled an out of date version then when I was looking over it today, sorry!
Going further this PR should
a) emit xz, y fields if specified in config, even if their values match. For this DespawnRange needs to track if the val was defined in long or short syntax. (Preferably new type that is Range(xz, y, longSyntax)
b) require defining vertical if horizonal was defined. No "partial" long syntax.