Re-login button not adjusting its position when resizing the window

[Bstn1802]

Expected behavior

Button should still be aligned with the back button

Observed/actual behavior

Button stays in place

Steps/models to reproduce

Get to a point where the re-login button shows on the disconnect screen and resize the window.

One of the quickest ways is to kick yourself from a singleplayer world in which case the re-login button also shows for some reason (another bug or intended? Didn't use to show in 1.19)

Version

7.0.2+1.20

Agreements

• I am running the latest version of the mod.
• My version of Minecraft is supported.
• I have searched for and ensured there isn't already an open issue regarding this.

Other

Obviously doesn't happen in the average use case. I only came across this while updating one of my own mods which also adds a button to the screen and I'm testing with this mod along side to make sure the buttons don't get all messed up.
The issue didn't happen in 1.19. Now the disconnect screen uses a grid widget to position all the drawable widgets. A possible fix might be to add the re-login button to the grid widget before it does its magic to position all its children, although I couldn't find out how exactly the widgets are repositioned now, I only assume that the grid widget might be responsible now. In older versions (I'm pretty sure) the init method just got called every time the window got resized, but this doesn't happen anymore, from what I can tell after some testing.

[Bstn1802]

After doing some testing with my mod, I figured out a way to add the button, which seems to work fine. I'm now injecting into the method call where the back button gets added instead of the method itself. Then instead of adding the back button I build and add another grid widget which I add both my own button and the back button into. The reason why I'm making another grid widget is because the existing one has a spacing of 20 pixels I think and the spacing between buttons is usually 4 pixels.
I would've made a pull request, but all the guidelines here kind of scare me to be honest and it's just a single method that would need to be changed. I quickly combined the working solution in my code with the current injection in your code and ended up with this piece of code, which I didn't really test, but it's still similar to my code which works fine, from the testing I did so far:

@ModifyArg(method = "init", index = 0, at = @At(value = "INVOKE", ordinal = 2, target = "Lnet/minecraft/client/gui/widget/GridWidget$Adder;add(Lnet/minecraft/client/gui/widget/Widget;)Lnet/minecraft/client/gui/widget/Widget;"))
private Widget initAddBackButton(Widget backButton) {
    // Determine if the disconnection reason is user or session related
    if (!isUserRelated(reason)) return;

    LOGGER.info("Adding auth button to the disconnected screen");
    assert client != null;

    // Create another grid widget with less spacing
    GridWidget grid = new GridWidget();
    grid.setRowSpacing(4);
    GridWidget.Adder adder = grid.createAdder(1);

    // Create and add the button to the screen where the back button is
    adder.add(
        ButtonWidget.builder(
            Text.translatable("gui.authme.button.relogin"),
            btn -> client.setScreen(new AuthMethodScreen(parent))
        ).build()
    );
    // Add the back button after that
    adder.add(backButton);

    // Edit: not necessary and would add elements repeatedly
    // grid.refreshPositions();
    // grid.forEachChild(this::addDrawableChild);

    return grid; // Return the grid to be added instead of the backButton
}

The way the grid widget is created and all the surrounding method calls are inspired by the way the grid widget in DisconnectScreen is handled.
Instead of a @ModifyArg a @Redirect could be used as well, which would also give you access to the adder and with that the grid widget from the disconnect screen. I used that before, but then realized I don't need it.
Us both adding our button(s) into the screen like that should also work fine, besides creating a not so nice but working nested hierarchy of grid widgets with each button. I found out that GridWidget extends LayoutWidget and that has the method refreshPositions which passes the call to nested layout widgets.
I further found out that resizing the window eventually triggers initTabNavigation which the disconnect screen overrides with passing an update on to the grid widget, which should confirm that this is working on the theory side of it too.

Edit: The calls I commented out are not necessary. I noticed that elements were appearing twice and on top of each other for me and then I remembered that layout widgets pass on the refreshPositions to layout children and noticed that the same applies for the forEachChild passing on to other children so those two lines should not be included since the outer most layout widget handles that already.

Edit2: I just tested a case where both of your buttons are added to the screen, which totally messes things up, since the way you retrieve the back button is quite hardcoded. The code snippet above wouldn't have that issue, but in case you want to handle this issue differently, you might want to consider retrieving the back button differently, since it's not always the third element, at least when other mods get involved adding buttons too.

Edit3: I named the argument backButton but technically you can not assume it is the back button, in case another mod injects there too, which is relevant for me at least, since I need to modify it too and if you decide to take my suggested approach it depends on which one of our mods gets to inject their code first, although probably not relevant for you and sorry by the way for the large amount of text.

[axieum]

Hi @Bstn1802, 👋

Apologies for the delay getting back to you.

This is great stuff, very detailed and I appreciate you taking the time to share your findings. I'm glad you've pointed out all the new widgets being used, I didn't think much of it when updating the mappings, so thank you!

From what you've shared, we need a way to replace the back button with a grid widget and share this around. Perhaps we could both add an instance check and accommodate the scenario where the grid widget already exists?

It might be worth exploring a way to do this through the widgets api, rather than mixins. I'll try to explore this avenue soon.

As for raising a PR, by all means go for it. The contribution guidelines mainly touch on the branching strategy and commit naming conventions. But the great thing is that maintainers will write the squashed merge commit message anyway. This is what determines the next version and what appears in the changelog.

I hope to have some time on Sunday to have a look at this issue.

Regards,
Jonathan.