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

Map item, warp command, poi command #58

Merged
merged 1 commit into from
Oct 27, 2023
Merged

Map item, warp command, poi command #58

merged 1 commit into from
Oct 27, 2023

Conversation

itsbazke
Copy link
Contributor

image

@itsbazke itsbazke self-assigned this Oct 27, 2023
.requires(source -> source.hasPermission(Commands.LEVEL_GAMEMASTERS))
.then(literal("edit")
.then(argument("name", word())
.suggests((ctx, builder) -> suggest(suggestName(ctx), builder))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for these suggestions id love to "remember" the initial POI somehow, is that possible?

src/main/java/com/lovetropics/extras/data/poi/Poi.java Outdated Show resolved Hide resolved
@itsbazke
Copy link
Contributor Author

itsbazke commented Oct 27, 2023

Should also figure out exactly how the PNG is made/exported and how to do it repeatedly, so it could get updated without messing up coords
updated map and added a little description

@itsbazke itsbazke marked this pull request as ready for review October 27, 2023 09:46
@Override
public InteractionResultHolder<ItemStack> use(final Level pLevel, final Player player, final InteractionHand usedHand) {
if (pLevel.isClientSide()) {
Minecraft.getInstance().setScreen(new TropicalMapScreen(Component.translatable("item.ltextras.tropical_map"), player));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modding setups are a bit weird where client & server source are merged - but the server still doesn't have the client classes available. This call to client-side code will need to be pulled into a separate static function, I think 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh confusing! something like this commit? 95429d2

return InteractionResultHolder.pass(player.getItemInHand(usedHand));
}

public static class TropicalMapScreen extends Screen {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These inner classes will also need to be moved out for classloading to not fail, afaik

//Dont think this is a great way to scale... just wanted the text a bit smaller :)
PoseStack pose = guiGraphics.pose();
pose.pushPose();
pose.scale(1.0f / FONT_SCALING, 1.0f / FONT_SCALING, 1.0f / FONT_SCALING);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a bit of a caveat here that text is the smallest size it can be to render correctly on all resolutions - at GUI scale 1, this will look a bit strange. One solution might be to only show the icon normally, and show the name when hovered? Imagining kind of like the advancements screen

But I think it's also not important - most players are using a scale bigger than 1 :D

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I'm not sure what is best to do here, here is some pictures:

No scaling
image

No scaling+larger icon
image

As is
image

Maybe the scaling should only be applied if the resolution is over some value? With scaling it doesnt look great in a small window, but in a large window it takes up a lot of space

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It becomes a bit hard to predict how things will look on different screen sizes & setups when it's dynamic - having it at a consistent scaling at least ensures that it will look the same for everyone, and match the configuration they have set with their GUI scale.
I don't think the big text + big icon looks too bad, I guess the problem might be running out of space?

@itsbazke itsbazke linked an issue Oct 27, 2023 that may be closed by this pull request
Copy link
Member

@Gegy Gegy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks great to me!

@itsbazke itsbazke force-pushed the map-item branch 2 times, most recently from dbf5593 to 97f26bc Compare October 27, 2023 14:20
@itsbazke itsbazke merged commit 3bdf55d into lt23 Oct 27, 2023
1 check passed
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.

Map item for teleporting
2 participants