-
-
Notifications
You must be signed in to change notification settings - Fork 30
Feat/astral map biome finder #1929
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
base: main
Are you sure you want to change the base?
Conversation
|
ive tested myself and it works and ive also gotten a beta tester and they had no issues |
|
closes #1698 |
drtheodor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
among other things, using ait:loading as a dummy value is rather silly. If it applies to an entire list can't you just make it a separate boolean value like "waitingOnBiomes"..?
src/main/java/dev/amble/ait/client/screens/AstralMapScreen.java
Outdated
Show resolved
Hide resolved
src/main/java/dev/amble/ait/client/screens/AstralMapScreen.java
Outdated
Show resolved
Hide resolved
src/main/java/dev/amble/ait/client/screens/AstralMapScreen.java
Outdated
Show resolved
Hide resolved
|
nitpicks done now approve so nano can do the gui |
| this.close(); | ||
| }); | ||
| waitingOnStructures = false; | ||
| changed = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed is already false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this supposed to be true?
| public static List<Identifier> biomeIds; | ||
|
|
||
| static { | ||
| ServerPlayNetworking.registerGlobalReceiver(REQUEST_SEARCH, (server, player, handler, buf, responseSender) -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can include what you're trying to query.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, this doesn't have a security check. what if the player isnt using an astral map block? hax
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can include what you're trying to query.
this is known in clientside code, and will make the server's job simpler
|
|
||
| private static void sendBiomes(ServerWorld world, ServerPlayerEntity target) { | ||
| if (biomeIds == null || biomeIds.isEmpty()) { | ||
| List<Identifier> ids = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be a static sized list or an array, iirc the registry's size can be queried
| } | ||
|
|
||
| PacketByteBuf buf = PacketByteBufs.create(); | ||
| buf.writeInt(biomeIds.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a writeCollection method already
| ids.add(buf.readIdentifier()); | ||
| } | ||
| client.execute(() -> { | ||
| biomeIds = ids; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add this.
| } | ||
| client.execute(() -> { | ||
| biomeIds = ids; | ||
| if (client.currentScreen instanceof dev.amble.ait.client.screens.AstralMapScreen screen) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import the class instead of querying it by its package name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, i feel like this can be cached client-side (if it isn't already present on client)
im pretty sure client knows of registered biomes... ill check the protocol wiki.
| RegistryKey<Biome> biomekey = RegistryKey.of(RegistryKeys.BIOME, target); | ||
|
|
||
| Pair<BlockPos, RegistryEntry<Biome>> r = targetWorld.locateBiome( | ||
| entry -> entry.getKey().map(key -> key.equals(biomekey)).orElse(false), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be replaced with RegistryEntryPredicateArgumentType.getRegistryEntryPredicate(context, "biome", RegistryKeys.BIOME)
| RegistryKey<Biome> biomekey = RegistryKey.of(RegistryKeys.BIOME, target); | ||
|
|
||
| Pair<BlockPos, RegistryEntry<Biome>> r = targetWorld.locateBiome( | ||
| entry -> entry.getKey().map(key -> key.equals(biomekey)).orElse(false), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be replaced with RegistryEntryPredicateArgumentType.getRegistryEntryPredicate(context, "biome", RegistryKeys.BIOME)
American.Psycho.Dry.Cleaners.If.you.don.t.shut.your.mouth.I.will.kill.you.Ayaa.S4r7og9JTJU.mp4 |
About the PR
astral map has biome locator
Why / Balance
suggestions from discord
Technical details
server config to change the block radius default value is 6400 which seems to work great, too low of a value like 256 will barely get you any results and im afraid too high of a value would be expensive to compute
Requirements
Changelog
🆑