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

Seed based feature search (Fixes #2312) #2852

Open
wants to merge 2 commits into
base: master
from

Conversation

@Phoenix616
Copy link
Contributor

Phoenix616 commented Jan 13, 2020

This fixes the issue where the server will load surrounding chunks up to a radius of 100 chunks in order to search for features e.g. when running the /locate command or for treasure maps (issue #2312).
This is done by using the same seed checking functionality that is used by the server when generating these features before actually attempting to load the chunk to check if a feature is available in it.

While the chunk loading itself will still be done on the main thread it should ideally only ever load a single chunk so this will be more compatible with existing usages of the feature search both in the server and the API. Theoretically this could be amended to also provide API methods to toggle this or to use the async chunk loading API but async chunk loading would require larger changes that require more looking into how to integrate into the rest of the server without breaking existing mechanics.

The only downside of this is that it breaks once the seed or generator changes but this should usually not happen. A config option to disable this improvement is added though in case that should ever be necessary.

}

+ public BiomeManager getBiomeManager() { return d(); } // Paper - OBFHELPER
@Override

This comment has been minimized.

Copy link
@Black-Hole

Black-Hole Jan 13, 2020

Contributor

Override should be moved to next line for effective use of the OPFHELPER.

This comment has been minimized.

Copy link
@Phoenix616

Phoenix616 Jan 13, 2020

Author Contributor

I'm unsure what you mean by that, the @Override is for the d() method which overrides the one in the IWorldReader interface. I guess you mean that the OBFHELPER should be in the interface via a default method or something like that?

@Black-Hole

This comment has been minimized.

Copy link
Contributor

Black-Hole commented Jan 13, 2020

I mean that you should combine the next two lines in one line, as there will never be a conflict triggered if the line below the Override changes.

@kickash32

This comment has been minimized.

Copy link
Contributor

kickash32 commented Jan 14, 2020

I would recommend a fallback config option, defaulting to true to retain vanilla behaviour. The fallback could work using a helper method that calls the search method 2x with a boolean seedSearch (once with true, if seed search is enabled and once false).

@Phoenix616

This comment has been minimized.

Copy link
Contributor Author

Phoenix616 commented Jan 14, 2020

I thought about this a bit more and I don't think that there should be a fallback or that it is even necessary to add it as an additional option.

  • If it would fall back to loading all chunks it would just end up crashing the server if it didn't find the feature because it really didn't exist there (e.g. by a fluke of the world generator or just because the biomes just aren't available). In such a case all 40,000 chunks would get loaded at once as it wouldn't find the feature. (Which would most likely lead to a crash which is why I don't think there should be a fallback)
  • And in the case where the seed or generator was changed and the user noticed that he could just disable the seed-based search. Providing a separate fallback option that can be enabled in that case would not make any sense either as it's almost 100% guaranteed that the seed-search would not find the feature in the first search, you would benefit more by just disabling the seed-based search.

Basically I can think of no situation where a fallback would actually be beneficial, especially as it would need to be disabled by default as explained in my first point.

@Callahhh

This comment has been minimized.

Copy link
Contributor

Callahhh commented Jan 20, 2020

Been using this on my ~35 player server, fixed the crashing completely. Haven't experienced any issues with it. Thanks : )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.