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

Upgrade to 1.18.2 #6

Closed
wants to merge 1 commit into from
Closed

Conversation

braxtonhall
Copy link

@braxtonhall braxtonhall commented May 13, 2022

resolves #4

Hello @Yunus1903 !

This branch successfully builds and has been tested on Minecraft 1.18.2.
I created this PR more as a proof of concept, as I have never written a Minecraft mod and really don't understand the ecosystem at all. There's a little bit of jank in here.

Please adjust the base branch when you create a 1.18 one

Copy link
Author

@braxtonhall braxtonhall left a comment

Choose a reason for hiding this comment

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

In this review is a little bit of commentary for the changes I made so far

@@ -1,46 +1,53 @@
import se.bjurr.gitchangelog.plugin.gradle.GitChangelogTask
Copy link
Author

Choose a reason for hiding this comment

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

This file was recreated by copying the base forge build.gradle and reintroducing function calls until things worked again. I assume I missed some important steps

@@ -1,78 +1,129 @@
#!/usr/bin/env sh
Copy link
Author

Choose a reason for hiding this comment

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

Gradle itself needed to be updated

@@ -0,0 +1,13 @@
package com.yunus1903.chatembeds.client;

public class ChatScrollPos {
Copy link
Author

Choose a reason for hiding this comment

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

This class will probably need to go.

I don't know how to access a private field in a Minecraft class, so this is used to get around chatScrollbarPos being private in ChatComponent

Copy link
Owner

Choose a reason for hiding this comment

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

You can transform private fields to be more permissive using Access Transformers.

{
if (lastScrollPos != mc.ingameGUI.getChatGUI().scrollPos)
if (lastScrollPos != getScrollPos())
Copy link
Author

Choose a reason for hiding this comment

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

This is the same hack mentioned above, chatScrollbarPos is private

{
index = 0;
}

@Inject(method = "resetChatScroll", at = @At("RETURN"))
public void resetChatScroll(CallbackInfo ci) {
setScrollPos(chatScrollbarPos);
Copy link
Author

Choose a reason for hiding this comment

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

Same hack mentioned above to expose chatScrollbarPos

minecraft.getTextureManager().bindTexture(embed.getFrames().get(currentFrame).getResourceLocation());
AbstractGui.blit(matrixStack, (width - scaledImageWidth) / 2,
RenderSystem.setShaderTexture(0, embed.getFrames().get(currentFrame).getResourceLocation());
RenderSystem.enableBlend();
Copy link
Author

Choose a reason for hiding this comment

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

to be honest I'm not even sure how this worked before. It really seems like this should have been required the whole time unless you were taking advantage of some temporal coupling.

@Yunus1903
Copy link
Owner

Yunus1903 commented May 13, 2022

Hi,

Cool to see you're interested and devoted to update the mod.
I wanted to do it myself as well but sadly haven't had time or motivation to do so.

I will try to review, cleanup and work on it as well in the next few days. Since I won't be having much time this weekend, most of it might happen next week.
I'll try to give as much feedback on here as possible and thanks for putting in time and effort to work on the mod 😅.

@Yunus1903 Yunus1903 self-assigned this May 13, 2022
@Yunus1903 Yunus1903 added the feature New feature or request label May 13, 2022
@Yunus1903 Yunus1903 changed the base branch from 1.16/dev to 1.18/dev May 13, 2022 03:37
@Yunus1903 Yunus1903 mentioned this pull request May 13, 2022
@braxtonhall
Copy link
Author

closing to get this out of my pull requests tab (I assume this will not get looked at again)

@braxtonhall braxtonhall closed this Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to 1.18
2 participants