MyreMylar / pygame_gui

A GUI system for pygame.
MIT License
698 stars 82 forks source link

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

Open LondonClass opened 8 months ago

LondonClass commented 8 months ago

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

GimLala commented 8 months ago

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 commented 8 months ago

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

GimLala commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

MyreMylar commented 7 months ago

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.