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

When using NaN as a default for an export inspector always shows reset to default arrow #50715

Open
t3nk3y opened this issue Jul 21, 2021 · 4 comments

Comments

@t3nk3y
Copy link

t3nk3y commented Jul 21, 2021

Godot version

Engine v4.0.dev.custom_build.3478690c0

System information

Ubuntu 21.04

Issue description

If an exported var is set with a default of NAN, the inspector in the editor will show the reset to default arrow, even when the value is set to NAN.

Steps to reproduce

This simply requires a derived class that can be edited in the inspector to have a default value of NAN.

extends Node

@export var this_is_nan = NAN

It will be displayed in editor with the reset arrow, even when it's already NAN
nan

Minimal reproduction project

No response

@lyuma
Copy link
Contributor

lyuma commented Jul 21, 2021

Your issue is likely caused by the definition of NaN. Note that NaN != NaN, that is to say, a NAN is never equal to itself (no NAN is equal to any other NAN). Hence, for a given scene, it seems reasonable to say that a given NAN value is not equal to the default NAN value, and thus a reset icon should show up. It's hard for me to see how this could be solved unless you add a special case for NaN, or compare serialized values, both of which sound like they would add performance overhead.

However, I believe Godot's current stance is that NAN and Infinity are not supported for anything that can get serialized.

I have found that, NaN and +/-Infinity do not serialize correctly in some cases. In particular, you will get different results when saving a resource/scene as text .tres/.tscn compared to saving in binary .res/.scn

Also, I think one case exists where an invalid value will cause a parse error when opening the resulting file.

@Calinou
Copy link
Member

Calinou commented Jul 22, 2021

I have found that, NaN and +/-Infinity do not serialize correctly in some cases. In particular, you will get different results when saving a resource/scene as text .tres/.tscn compared to saving in binary .res/.scn

We should probably fix this to avoid data loss 🙂

I'm not sure what would be the best solution with regards to reliability and implementation complexity:

  • Allow INF/NAN and treat them normally.
  • Save INF/NAN values as 0.0 instead (or perhaps 1.0 to avoid division by zero, if it's used somewhere as a divisor).
  • Display an error message if the scene is saved but it contains INF/NAN values (only possible if safe writing is enabled).

@t3nk3y
Copy link
Author

t3nk3y commented Jul 22, 2021

I was using NAN here because I want my float's to have a default "empty" state, and NAN or INF seemed to be the only solution.

I think storage would require a unique NAN symbol. But, in this particular case, NAN is default, so nothing needs saved with the TSCN. Defaults are not saved, right?

I think, code wise in the editor, basically you would also need:

if is_nan(prop) and is_nan(default):
    is_default = true
if prop == default:
    is_default = true

@MrEgggga
Copy link

MrEgggga commented Jan 29, 2024

Starting with #81076 (I think), export fields with default value NaN not only always show a reset arrow but display as 0 (while being stored as nan).

I'm not sure what would be the best solution with regards to reliability and implementation complexity:

* Allow INF/NAN and treat them normally.

* Save INF/NAN values as `0.0` instead (or perhaps `1.0` to avoid division by zero, if it's used somewhere as a divisor).

* Display an error message if the scene is saved but it contains INF/NAN values (only possible if safe writing is enabled).

In my opinion, the first option here is probably the best one. NaN and infinity are legitimate float values with some potential use cases so it seems strange to intentionally disallow their use either by storing them as other numbers or by preventing scenes including INF/NAN from being saved.

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

4 participants