godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.23k stars 21.04k forks source link

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

Closed ParadoxV5 closed 1 year ago

ParadoxV5 commented 1 year ago

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: https://github.com/godotengine/godot/blob/9f12e7b52d944281a39b7d3a33de6700c76cc23a/core/extension/gdextension_interface.h#L119 API JSON: https://github.com/ParadoxV5/godot-headers/blob/732ed57b5f19583a7af4272e58ce509d5ff58586/extension_api.json#L3985

Steps to reproduce

Minimal reproduction project

N/A

Calinou commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

https://github.com/godotengine/godot/blob/116f783db73f4bf7e9e96ae54dd3d0a20337cc8a/core/variant/variant.h#L522

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.

ParadoxV5 commented 1 year ago

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.)