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

GDExtension API: Modulo operator’s enum is misnamed OP_MODULE #76588

Closed
ParadoxV5 opened this issue Apr 29, 2023 · 4 comments
Closed

GDExtension API: Modulo operator’s enum is misnamed OP_MODULE #76588

ParadoxV5 opened this issue Apr 29, 2023 · 4 comments

Comments

@ParadoxV5
Copy link

ParadoxV5 commented Apr 29, 2023

Godot version

as early as 3.1 – 4.0.2

System information

GDExtension (also applies to GDNative)

Issue description

Moved and elaborated from godotengine/godot-headers#105

The modulo operator’s enum in GDExtension/GDNative is named OP_MODULE rather than OP_MODULO.

C header:

GDEXTENSION_VARIANT_OP_MODULE,

API JSON: https://github.com/ParadoxV5/godot-headers/blob/732ed57b5f19583a7af4272e58ce509d5ff58586/extension_api.json#L3985

  • If this is not intended, a change for the next minor patch is much appreciated.
  • If this is intentional (e.g., header naming limitation), please document the reason either near the header line or on Godot docs and link.
  • If this is not intended but was left as is simply for backward compatibility, then it’s unfortunate that the Godot 4.0 overhaul overlooked it, but it deserves consideration for the hypothetical future version 5.0. (I searched the tracker with the keyword ‘modulo’ already and there wasn’t a previous issue for project tracking.)

Steps to reproduce

  • C header: check the core/extension/gdextension_interface.h file at around line 119
  • API JSON: generate the file and look at around line 3985

Minimal reproduction project

N/A

@Calinou
Copy link
Member

Calinou commented Apr 29, 2023

If this is not intended but was left as is simply for backward compatibility, then it’s unfortunate that the Godot 4.0 overhaul overlooked it, but it deserves consideration for the hypothetical future version 5.0. (I searched the tracker with the keyword ‘modulo’ already and there wasn’t a previous issue for project tracking.)

@ParadoxV5 ParadoxV5 moved this to 🗳️ Submitted in General Projects Apr 29, 2023
@ParadoxV5
Copy link
Author

it’s unfortunate that the Godot 4.0 overhaul overlooked it

Just came across #16863. OP_MODULEOP_MODULO is not on final list.

@akien-mga
Copy link
Member

The GDExtension API reflects what's in the core Variant API:

OP_MODULE,

Changing this would break compatibility, which we don't want to do before Godot 5 unless it's absolutely needed to fix critical issues. This slight misnomer doesn't seem critical to fix now.

@akien-mga akien-mga closed this as not planned Won't fix, can't repro, duplicate, stale Jun 19, 2023
@ParadoxV5
Copy link
Author

Changing this would break compatibility, which we don't want to do before Godot 5 unless it's absolutely needed to fix critical issues. This slight misnomer doesn't seem critical to fix now.

If this is not intended but was left as is simply for backward compatibility, then it’s unfortunate that the Godot 4.0 overhaul overlooked it, but it deserves consideration for the hypothetical future version 5.0. (I searched the tracker with the keyword ‘modulo’ already and there wasn’t a previous issue for project tracking.)

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

3 participants