Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Configuration for vertical Despawn Ranges #10440

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

notTamion
Copy link
Contributor

@notTamion notTamion commented Apr 20, 2024

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

@notTamion notTamion requested a review from a team as a code owner April 20, 2024 15:28
@notTamion notTamion marked this pull request as draft April 20, 2024 15:33
@notTamion notTamion marked this pull request as ready for review April 20, 2024 15:49
@Machine-Maker
Copy link
Member

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

@notTamion
Copy link
Contributor Author

is there an existing way i can add optional configurations like that?

@lynxplay
Copy link
Contributor

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.

@notTamion
Copy link
Contributor Author

alright i will probably be working on the custom serializer and the rewrite tomorrow

@notTamion notTamion force-pushed the add-config-for-y-despawn-ranges branch 2 times, most recently from d6f0c44 to 386f521 Compare April 21, 2024 06:13
@notTamion
Copy link
Contributor Author

Implemented the requested changes

@notTamion notTamion force-pushed the add-config-for-y-despawn-ranges branch from 862c67c to 679ed07 Compare April 28, 2024 09:13
@notTamion notTamion changed the title Add Configuration for y Despawn Ranges Add Configuration for vertical Despawn Ranges Apr 28, 2024
@notTamion notTamion force-pushed the add-config-for-y-despawn-ranges branch from fba9cdc to 036ccbd Compare April 28, 2024 12:39
@notTamion notTamion force-pushed the add-config-for-y-despawn-ranges branch from 036ccbd to 272c867 Compare May 13, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a separate Y-value check for "despawn-ranges"
3 participants