pygame-community / pygame-ce

🐍🎮 pygame - Community Edition is a FOSS Python library for multimedia applications (like games). Built on top of the excellent SDL library.
https://pyga.me
766 stars 120 forks source link

Frect setting side to another side inconsistent #2909

Closed ThisProgrammerG closed 3 weeks ago

ThisProgrammerG commented 3 weeks ago

Environment:

pygame-ce 2.4.1 (SDL 2.28.5, Python 3.12.2)
Platform:       Windows-10-10.0.19045-SP0
System:         Windows
System Version:     10.0.19045
Processor:      AMD64 Family 23 Model 1 Stepping 1, AuthenticAMD
Architecture:       Bits: 64bit Linkage: WindowsPE
Driver:         Display Not Initialized

Python:         CPython
pygame version:     2.4.1
python version:     3.12.2

SDL versions:       Linked: 2.28.5  Compiled: 2.28.5
SDL Mixer versions: Linked: 2.8.0   Compiled: 2.8.0
SDL Font versions:  Linked: 2.22.0  Compiled: 2.22.0
SDL Image versions: Linked: 2.8.2   Compiled: 2.8.2
Freetype versions:  Linked: 2.11.1  Compiled: 2.11.1

Current behavior: Using FRects: Setting a side ["top", "bottom", "left", "right"] to another side are not exactly the same via:

frect.bottom = other.bottom 
assert frect.bottom == other.bottom # Sometimes fails

Bug happens after changing the value of the points ["topleft", "topright", "bottomright", "bottomleft", "center"].

Expected behavior:

When setting one side to another they should be exactly the same.

Steps to reproduce: Change the value of mentioned points then set the side value to another.

Test code

import random
from itertools import combinations_with_replacement

import pygame

class Obstacle:
    def __init__(self, position, size):
        self.image = pygame.Surface(size)
        self.rect = self.image.get_frect(topleft=position)

def test_frect_point(rect_point, obstacle, player, velocity, bound=1.0, speed=400, delta_time=0.001):
    side_combinations = combinations_with_replacement(["top", "bottom", "left", "right"], r=2)
    fail_sides = {sides: 0 for sides in side_combinations}
    runs = 10_000

    for sides in fail_sides.keys():
        for _ in range(runs):
            velocity.update(random.uniform(-bound, bound), random.uniform(-bound, bound))
            delta_velocity = velocity * speed * delta_time
            setattr(obstacle.rect, rect_point, getattr(obstacle.rect, rect_point) + delta_velocity)
            setattr(player.rect, sides[0], getattr(obstacle.rect, sides[1]))
            if getattr(player.rect, sides[0]) != getattr(obstacle.rect, sides[1]):
                fail_sides[sides] += 1

    return fail_sides

def display_results(fail_sides, rect_point):
    header = f"Changing {rect_point} ============"
    sorted_by_most_fails = dict(sorted(fail_sides.items(), key=lambda item: item[1], reverse=True))
    text_width = max(len(str(sides)) for sides in sorted_by_most_fails.keys())

    print(header)
    for sides, count in sorted_by_most_fails.items():
        print(f"{sides}{(text_width - len(str(sides))) * ' '}: {count}")

def main():
    obstacle = Obstacle((25, 25), (500, 1000))
    player = Obstacle((100, 100), (50, 10))
    velocity = pygame.Vector2()

    for point in ["topleft", "topright", "bottomright", "bottomleft", "center"]:
        results = test_frect_point(point, obstacle, player, velocity)
        display_results(results, point)

if __name__ == '__main__':
    main()

My Output

Changing topleft ============
('bottom', 'right') : 9723
('top', 'bottom')   : 9694
('bottom', 'bottom'): 9560
('top', 'right')    : 9064
('left', 'right')   : 8832
('right', 'right')  : 8529
('bottom', 'left')  : 3235
('top', 'top')      : 0
('top', 'left')     : 0
('left', 'left')    : 0
Changing topright ============
('bottom', 'bottom'): 9605
('top', 'bottom')   : 9585
('top', 'top')      : 0
('top', 'left')     : 0
('top', 'right')    : 0
('bottom', 'left')  : 0
('bottom', 'right') : 0
('left', 'left')    : 0
('left', 'right')   : 0
('right', 'right')  : 0
Changing bottomright ============
('top', 'top')      : 0
('top', 'bottom')   : 0
('top', 'left')     : 0
('top', 'right')    : 0
('bottom', 'bottom'): 0
('bottom', 'left')  : 0
('bottom', 'right') : 0
('left', 'left')    : 0
('left', 'right')   : 0
('right', 'right')  : 0
Changing bottomleft ============
('bottom', 'right') : 8775
('top', 'right')    : 8764
('left', 'right')   : 8407
('right', 'right')  : 7478
('top', 'top')      : 0
('top', 'bottom')   : 0
('top', 'left')     : 0
('bottom', 'bottom'): 0
('bottom', 'left')  : 0
('left', 'left')    : 0
Changing center ============
('left', 'right')   : 5076
('top', 'bottom')   : 5010
('bottom', 'right') : 4984
('right', 'right')  : 4946
('top', 'right')    : 4935
('bottom', 'bottom'): 2835
('top', 'top')      : 0
('top', 'left')     : 0
('bottom', 'left')  : 0
('left', 'left')    : 0

Corner "bottomright" has no errors from my runs. Link to Reddit post: https://new.reddit.com/r/pygame/comments/1d7skor/setting_frect_edge_to_another_inconsistent/

oddbookworm commented 3 weeks ago

As @Starbuck5 mentioned in the reddit post, this is due to conversion between C float and C double (Python float). I've created this reproducer script which outputs a file of differences, in bit representations.

import pygame
import random
import struct

def binary_float(num):
    return ''.join('{:0>8b}'.format(c) for c in struct.pack('!f', num))

def binary_double(num):
    return ''.join('{:0>8b}'.format(c) for c in struct.pack('!d', num))

# ensure file is empty
with open("out.txt", "w") as outFile:
    pass

def get_random_number() -> float:
    return random.randint(0, 100) + random.random()

def create_rects_and_compare() -> None:
    r1 = pygame.FRect(get_random_number(), get_random_number(), get_random_number(), get_random_number())
    r2 = pygame.FRect(get_random_number(), get_random_number(), get_random_number(), get_random_number())

    r1.topleft = get_random_number(), get_random_number()
    r2.center = r1.center

    if r1.center != r2.center:
        with open("out.txt", "a") as outFile:
            outFile.write("bit breakdown:\n\tAs doubles:")
            outFile.write(f"\n\t\t{binary_double(r1.center[0])}, {binary_double(r1.center[1])}")
            outFile.write(f"\n\t\t{binary_double(r2.center[0])}, {binary_double(r2.center[1])}")
            outFile.write("\n\tAs floats:")
            outFile.write(f"\n\t\t{binary_float(r1.center[0])}, {binary_float(r1.center[1])}")
            outFile.write(f"\n\t\t{binary_float(r2.center[0])}, {binary_float(r2.center[1])}\n")

for _ in range(10_000):
    create_rects_and_compare()
oddbookworm commented 3 weeks ago

Here's a sample form one of my runs:

bit breakdown:
    As doubles:
        0100000000110001011111101001101011100000000000000000000000000000, 0100000001010111101011000110111101000000000000000000000000000000
        0100000000110001011111101001101011000000000000000000000000000000, 0100000001010111101011000110111101000000000000000000000000000000
    As floats:
        01000001100010111111010011010111, 01000010101111010110001101111010
        01000001100010111111010011010110, 01000010101111010110001101111010
bit breakdown:
    As doubles:
        0100000001010001010100010111101111000000000000000000000000000000, 0100000001001100101100010100011001000000000000000000000000000000
        0100000001010001010100010111101110100000000000000000000000000000, 0100000001001100101100010100011001000000000000000000000000000000
    As floats:
        01000010100010101000101111011110, 01000010011001011000101000110010
        01000010100010101000101111011101, 01000010011001011000101000110010
bit breakdown:
    As doubles:
        0100000001010010101011000111111101000000000000000000000000000000, 0100000001000000101010101101001011100000000000000000000000000000
        0100000001010010101011000111111101000000000000000000000000000000, 0100000001000000101010101101001011000000000000000000000000000000
    As floats:
        01000010100101010110001111111010, 01000010000001010101011010010111
        01000010100101010110001111111010, 01000010000001010101011010010110
bit breakdown:
    As doubles:
        0100000001001000111111100011110100000000000000000000000000000000, 0100000001010011011110011110001111000000000000000000000000000000
        0100000001001000111111100011110100100000000000000000000000000000, 0100000001010011011110011110001111000000000000000000000000000000
    As floats:
        01000010010001111111000111101000, 01000010100110111100111100011110
        01000010010001111111000111101001, 01000010100110111100111100011110

As can be seen, the differences really only show up at around the 30ish bit mark, which is about where the limits of float accuracy is. The problem is that trunctation has to happen when downcasting from double to float, and that's not always a clean truncation to perform

oddbookworm commented 3 weeks ago

I never recommend comparing floating point numbers exactly for the very reason that it's basically impossible to get them exactly the same. Usually, I recommend having a tolerance within which they're considered "equal". In pygame vectors for example, that tolerance is built into the comparison of the vector objects themselves with the epsilon value. It defaults to 1e-6, so any two vectors will compare equal if their components are within 1e-6 of each other

ankith26 commented 3 weeks ago

Hello! I think there is nothing to be fixed here. As oddbookworm has mentioned, it is a bad idea to rely on "float equality comparisons" because of the nature of floats. You may know the classic 0.1 + 0.2 != 0.3

It is also not viable to make FRect use double instead of float, because that decision was already made upstream by SDL and we are just binding it to python code.