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

Inconsisten Time error handling #80059

Closed
acgc99 opened this issue Jul 30, 2023 · 7 comments
Closed

Inconsisten Time error handling #80059

acgc99 opened this issue Jul 30, 2023 · 7 comments

Comments

@acgc99
Copy link

acgc99 commented Jul 30, 2023

Godot version

v4.1.stable.official [9704596]

System information

Godot v4.1.stable - Windows 10.0.22621 - Vulkan (Mobile) - dedicated NVIDIA GeForce GTX 1050 (NVIDIA; 31.0.15.3667) - Intel(R) Core(TM) i5-8300H CPU @ 2.30GHz (8 Threads)

Issue description

Error handling is not well implemented. Try this:

print("Invalid date")
print(Time.get_unix_time_from_datetime_string("1970-01-40"))
print("Unix 0")
print(Time.get_date_string_from_unix_time(0))
print("From string")
print(Time.get_unix_time_from_datetime_string("1970-01-01"))

You will get:

Invalid date
0
Unix 0
1970-01-01
From string
0

Furthermore, in the global scope, OK = 0.

Conclusion, you can't distinguish between an invalid date and date 1970-01-01.

Steps to reproduce

See "Issue description"

Minimal reproduction project

N/A

@AThousandShips
Copy link
Member

AThousandShips commented Jul 30, 2023

Furthermore, in the global scope, OK = 0.

It doesn't return an error value so this isn't really relevant.

Since returning any other value would break compatibility (as it isn't necessarily clear that it is an error) I'd suggest documenting this detail

@acgc99
Copy link
Author

acgc99 commented Jul 30, 2023

I would suggest:

  • To raise an error that stops execution when an invalid date is given.
  • Implement a method that returns OK if the date is valid and FAILED otherwise.

The user would have the option to manage the error with these modifications.

@AThousandShips
Copy link
Member

AThousandShips commented Jul 30, 2023

To raise an error that stops execution when an invalid date is given.

Would break compatibility

Implement a method that returns OK if the date is valid and FAILED otherwise.

Doesn't work since GDScript can't return by reference, so you can't return the value and the error

The current behavior is by design, see #60215, returning -1 if the format is wrong, and 0 if the date itself is invalid

@acgc99
Copy link
Author

acgc99 commented Jul 30, 2023

Doesn't work since GDScript can't return by reference, so you can't return the value and the error

The function that checks the date wouldn't have to raise any error, just determine if the date is valid. With this, the user can decide if to call other Time methods.

@AThousandShips
Copy link
Member

AThousandShips commented Jul 30, 2023

That would be the area of a proposal, so please open one for that if you like (make sure you follow all the steps and fill in everything)

@acgc99
Copy link
Author

acgc99 commented Jul 30, 2023

I opened a bug because you can't distinguish between an invalid date and converting to unix 1970-01-01.

You can see my proposal here.

@AThousandShips
Copy link
Member

Closing in favour of godotengine/godot-proposals#7414 (comment) as the current behaviour is by design (and would break compatibility)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants