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

Big Anchor Refactor: Dynamic dimension elements with anchors appear to calculate position before dimensions #515

Open
LondonClass opened this issue Feb 15, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@LondonClass
Copy link
Contributor

LondonClass commented Feb 15, 2024

Describe the bug
Anchors have different behaviors for static and dynamic elements.

To Reproduce

from pygame import Rect
from pygame_gui import UIManager
from pygame_gui.elements import UIButton


pygame.init()

pygame.display.set_caption('Quick Start')
window_surface = pygame.display.set_mode((800, 600))
manager = UIManager((800, 600))

background = pygame.Surface((800, 600))
background.fill((50, 50, 50))

clock = pygame.time.Clock()
is_running = True

window = pygame_gui.elements.UIWindow(rect=pygame.Rect((50, 50), (600, 300)),
                                                        manager=manager, resizable=True,
                                                        window_display_title='Static Dimensions')
hello_button = UIButton(Rect(-300, -150, -1, -1), 'Hello', container=window,
                                  anchors={'top': 'bottom',
                                           'left': 'right',
                                           'bottom': 'bottom',
                                           'right': 'right'})  # button that starts with dynamic dimensions
hello_button_2 = UIButton(Rect(-300, -150, hello_button.rect.width, hello_button.rect.height), 'Hello', container=window,
                                  anchors={'top': 'bottom',
                                           'left': 'right',
                                           'bottom': 'bottom',
                                           'right': 'right'})  # button starts with static dimensions

while is_running:
    time_delta = clock.tick(60)/1000.0
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            is_running = False

        manager.process_events(event)

    manager.update(time_delta)

    window_surface.blit(background, (0, 0))
    manager.draw_ui(window_surface)

    pygame.display.update()

Expected behaviour
The two buttons overlap.
I prefer the syntax of dynamic rectangles, so we don't have to subtract their width and height when passing relative-rect. So I hope to modify the anchor position of the static rectangle to match the dynamic rectangle.

Screenshots
T3H51YJ_9ITNJ3ADAV3PQ J

@LondonClass LondonClass added the bug Something isn't working label Feb 15, 2024
@GimLala
Copy link
Contributor

GimLala commented Feb 18, 2024

I think it's a pretty easy fix where when the elements are anchored to the right, we just subtract the width automatically while calculating the absolute rect, but the main problem is backwards compatibility. However, I think it's a necessary change because it will make the library more intuitive for new-comers.

@MyreMylar
Copy link
Owner

This seems more like a bug with the dynamic element to me, it looks like it is calculating the position of the left hand side before calculating the width of the button.

You can check by creating a non-anchored button where the left edge is 300 pixels in from the edge of the container:

import pygame
import pygame_gui
from pygame import Rect
from pygame_gui import UIManager
from pygame_gui.elements import UIButton

pygame.init()

pygame.display.set_caption("Quick Start")
window_surface = pygame.display.set_mode((800, 600))
manager = UIManager((800, 600))

background = pygame.Surface((800, 600))
background.fill((50, 50, 50))

clock = pygame.time.Clock()
is_running = True

window = pygame_gui.elements.UIWindow(
    rect=pygame.Rect((50, 50), (600, 300)),
    manager=manager,
    resizable=True,
    window_display_title="Static Dimensions",
)
hello_button = UIButton(
    Rect(-300, -150, -1, -1),
    "Hello 1",
    container=window,
    anchors={"top": "bottom", "left": "right", "bottom": "bottom", "right": "right"},
)  # button that starts with dynamic dimensions
print(hello_button.rect)
hello_button_2 = UIButton(
    Rect(
        -300,
        -150,
        hello_button.rect.width,
        hello_button.rect.height,
    ),
    "Hello 2",
    container=window,
    anchors={"top": "bottom", "left": "right", "bottom": "bottom", "right": "right"},
)  # button starts with static dimensions

hello_button_3 = UIButton(
    Rect(
        window.get_container().get_rect().width - 300,
        window.get_container().get_rect().height - 100,
        hello_button.rect.width,
        hello_button.rect.height,
    ),
    "Hello 3",
    container=window,
)  # button starts with static dimensions

while is_running:
    time_delta = clock.tick(60) / 1000.0
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            is_running = False

        manager.process_events(event)

    manager.update(time_delta)

    window_surface.blit(background, (0, 0))
    manager.draw_ui(window_surface)

    pygame.display.update()

Produces:

image

@MyreMylar MyreMylar changed the title Anchors have different behaviors for static and dynamic elements Dynamic dimension elements with anchors appear to calculate position before dimensions Feb 26, 2024
@GimLala
Copy link
Contributor

GimLala commented Mar 1, 2024

Yeah, I agree it is a bug with the dynamic elements, but wouldn't it be nice if when you anchored the right edge of the element to the right, you wouldn't have to subtract the width, but you would have to do it if left was anchored to the right? Well, it's your call in the end but I've noticed new users finding the current implementation unintuitive (me too when I just started using the library)

@LondonClass
Copy link
Contributor Author

Yeah, I agree it is a bug with the dynamic elements, but wouldn't it be nice if when you anchored the right edge of the element to the right, you wouldn't have to subtract the width, but you would have to do it if left was anchored to the right? Well, it's your call in the end but I've noticed new users finding the current implementation unintuitive (me too when I just started using the library)

For dynamic elements, binding them with 0,0 in the bottom right corner is a good choice. For dynamic elements, their size is unknown. If users need to calculate it themselves, it would be too troublesome.

@MyreMylar
Copy link
Owner

The main issue is that pygame Rectangles are defined like this:

pygame.Rect(top_left, dimensions)

So the first parameter always indicates a single point where the top and left of the rectangle will be.

With the anchors right now I am maintaining this single point as the reference on the object but allowing us to change what the offset to the top left is from. Normally in pygame the top left corner of a rectangle is always offset from the top left corner of the screen/containing surface, but it is fairly normal to have negative positions above the top left corner of the screen/surface or to the left of it - e.g. to indicate an object is off screen but may come onscreen later.

When building the GUI library I've been trying to keep these pygame conventions in mind which is why it works as it does right now, there is a straight mapping between how pygame.Rect rectangles work in pygame and how all the GUI elements do their positioning.

I can see the issue that you are trying to solve here - wanting to set a simple gap between the right hand side of a button, for example, and the side of a container. e.g.

image

To create that you want to do something like:

UIButton(relative_rect=Rect(-10, -10, 80, 40), text="OK", anchors={"bottom": "bottom", "right": "right"})

Instead of:

UIButton(relative_rect=Rect(-90, -50, 80, 40), text="OK", anchors={"bottom": "bottom", "right": "right"})

However, pygame rectangles already let you set the position of the rectangle by any of the points or sides. So I would suggest doing this:

UIButton(Rect(0, 0, 80, 40).move_to(bottomright=(-10, -10)), "OK", anchors={"bottom": "bottom", "right": "right"})

For a reasonably compact syntax that doesn't break anything and doesn't require you to do any calculation.

@LondonClass
Copy link
Contributor Author

However, pygame rectangles already let you set the position of the rectangle by any of the points or sides. So I would suggest doing this:

UIButton(Rect(0, 0, 80, 40).move_to(bottomright=(-10, -10)), "OK", anchors={"bottom": "bottom", "right": "right"})

For a reasonably compact syntax that doesn't break anything and doesn't require you to do any calculation.

This approach will fail when dealing with dynamic elements. Because the width and height of dynamic elements are -1 at this time, they cannot be calculated correctly.

With the anchors right now I am maintaining this single point as the reference on the object but allowing us to change what the offset to the top left is from. Normally in pygame the top left corner of a rectangle is always offset from the top left corner of the screen/containing surface, but it is fairly normal to have negative positions above the top left corner of the screen/surface or to the left of it - e.g. to indicate an object is off screen but may come onscreen later.

Anchoring the center seems to violate the principle. The parameter points to the center of the rectangle.

Thank you for the explanation. I think I know how to do it. I think the correct input of "relative_rect" should be fixed to the top left corner to maintain logical consistency. Provide a new positional input for creating elements. The new input is the absolute distance to the anchored side, which makes it more convenient to create elements with fixed margins. Set rect as an optional parameter, and the input positional parameter can be selected between the two, or simply determine which positional parameter is input based on the input type.

@LondonClass
Copy link
Contributor Author

Additionally, I would like to extract the position related parts of the elements into new classes.

@MyreMylar
Copy link
Owner

Just linking this to #506 And I'm going to rename it 'big anchor refactor'. I believe once we have an 'anchor_source' type variable it will be possible to resolve the issue with dynamic width elements satisfactorily.

@MyreMylar MyreMylar added the enhancement New feature or request label Apr 13, 2024
@MyreMylar MyreMylar changed the title Dynamic dimension elements with anchors appear to calculate position before dimensions Big Anchor Refactor: Dynamic dimension elements with anchors appear to calculate position before dimensions Apr 13, 2024
@MyreMylar MyreMylar removed the bug Something isn't working label Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants