godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

Improve usability of bitflag fields in GDScript #923

Open willnationsdev opened 4 years ago

willnationsdev commented 4 years ago

Reproduction of godotengine/godot#17708

Describe the project you are working on: RPG gameplay systems that involve defining bit flag enum values.

Describe the problem or limitation you are having in your project: While it is efficient and useful to work with bit flag enums, inexperienced programmers (and even me sometimes) can find it difficult to parse code that uses bitwise operations to modify and/or evaluate the state of those enums.

Describe the feature / enhancement and how it helps to overcome the problem or limitation: There should be a more human-readable API for working with bit flag enums in GDScript:

(This interface is courtesy of @romlok who commented on the original issue)

bitflag AttackType {
    ATTACK_PHYSICAL,
    ATTACK_FIRE,
    ATTACK_COLD,
    ATTACK_ACID,
}
# Similar to an enum, this would create:
const ATTACK_PHYSICAL = 1
const ATTACK_FIRE = 2
const ATTACK_COLD = 4
const ATTACK_ACID = 8
# And the AttackType object, where you can reference the values
var my_attack = AttackType.ATTACK_PHYSICAL | AttackType.ATTACK_FIRE

# BUT ALSO!
# Can be instanced to expose meaningfully-named methods
my_attack = AttackType.new() # or maybe `AttackType()`?
my_attack = my_attack.set(ATTACK_PHYSICAL)
my_attack = my_attack.clear(ATTACK_FIRE)
my_attack = my_attack.toggle(ATTACK_ACID)
var is_frosty = my_attack.has(ATTACK_COLD)
var actual_int_value = my_attack.value
# And maybe
if ATTACK_FIRE in my_attack:
    burn_stuff()

The .set(), .clear(), .toggle(), and .has() methods would all be vararg operations that accept a comma-separated list of potential things to mutate/check.

Describe how your proposal will work, with code, pseudocode, mockups, and/or diagrams:

  1. Introduce new keyword in GDScript to differentiate typical enums from bitflag enums
  2. These would create a GDScript-specific backend class instance, similar to GDNativeClass (perhaps GDBitFlag?), that has an alternative API, but which evaluates as an int when prompted.

Of course, these would only exist within GDScript code, so if you exported one of these, it would be exactly like if you'd exported a regular enum with the FLAGS hint.

If this enhancement will not be used often, can it be worked around with a few lines of script?: I would use it regularly, as I'm sure others would too. It can be worked around with a few lines of script (since you can resort to bitwise operations for all of this), but the usability and readability would suffer which is exactly the problem this proposal wants to resolve.

Is there a reason why this should be core and not an add-on in the asset library?: It would be possible for you to simply create a BitFlag Resource script that has most of these features, and then have exporting it generate an interface similar to that of the existing enum flag export. It's just that, the whole point is to improve the usability of GDScript as a language. And the entire selling point of GDScript is its usability, so to leave this idea as an addon rather than a built-in feature kind of defeats the point.

Xrayez commented 4 years ago

Marginally related but I used those bit flag enums in GDScript by bit shifting: godotengine/godot#23843, this feature got regressed in 3.2 but then fixed in 4.0... So I guess I'd find this proposal useful, I'm not sure whether this is relevant with GDScript 2.0.

Calinou commented 2 years ago

https://github.com/godotengine/godot/pull/62374 should improve the documentation side of this already. That said, since enums remain integers in GDScript (and can't have methods attached to them), adding bitfield helper methods could probably only be done by adding global scope methods.

Calinou commented 1 year ago

4.0 already exposes bitfields, but there is no way to define them in GDScript. I suppose adding a @bitfield annotation you can add in front of enum makes the most sense, as this is mostly for documentation purposes.

willnationsdev commented 1 year ago

@Calinou If we added #2816, then the engine could actually define a built-in struct Enum that wraps the integer value and abstracts away operations involving it using the proposal's accompanying struct "extension methods."

You could then end up with something like this maybe?


@bitfield # as you suggest, would just automate 2^n bit-flag initialization of the enum values
enum AttackType {
    ATTACK_PHYSICAL,
    ATTACK_FIRE,
    ATTACK_COLD,
    ATTACK_ACID,
}

func _ready():
    var my_attack = Enum(AttackType)
    my_attack = my_attack.set(ATTACK_PHYSICAL)
    my_attack = my_attack.clear(ATTACK_FIRE)
    my_attack = my_attack.toggle(ATTACK_ACID)
    var is_frosty = my_attack.has(ATTACK_COLD)
    var actual_int_value = my_attack.value
anvilfolk commented 1 year ago

Been thinking about this one after it was requested again on Reddit and since it seems to have a fair amount of support! :)

I'm having a hard time seeing something like my_attack = my_attack.toggle(ATTACK_ACID) working without adding an entirely new bitfield type for GDScript.

Here's a sketch of an alternative proposal:

  1. Add an is_enum_bitfield flag to GDScriptParser::DataType.
  2. If needed, we can add is_enum_bitfield to that to the dictionary of the enums' reduced_value, allowing runtime checks for whether a dictionary is a bitfield. I'm not certain this would be needed, though perhaps it is useful for runtime introspection, e.g. "SomeRandomEnum.is_enum_bitfieldorSomeRandomEnum["is_enum_bitfield"]`.
  3. Add GlobalScope functions for bitfield operations: bitfield_toggle/set/clear(my_attack, ATTACK_FIRE) feels more possible than my_attack.toggle(ATTACK_FIRE), since it doesn't require my_attack to have the method toggle - which it doesn't, since it's effectivelyint. We could hardcode my_attack.toggle/set/clear() into the GDScript analyzer/compiler, but that feels ad-hoc, unprincipled and harder to maintain (although it would allow typechecking).
  4. This allows my_attack to remain an int, so you can still do bitwise operations on it and other int things, and ignore globalscope bitfield functions.
  5. Depending on whether an exported variable is a regular enum or a bitfield, the editor could use a drop-down like it does for enums now, or a list of checkboxes like it does for @export_flags, which would effectively become slightly redundant.

I don't immediately know that we can do proper type checking with a globalscope bitfield_toggle/set/clear, but it might be possible. Will keep thinking about it :)

dalexeev commented 1 year ago

See also godotengine/godot#71234.

anvilfolk commented 1 year ago

@sp3llwe4ver was the person who mentioned this on Reddit, actually! :) Just pinged them there as well.

Will edit the above proposal to address the editor concerns as well, which I totally forgot. I think generally this proposal would mostly make @export_flags deprecated?

vnen commented 1 year ago

@export_flags is still useful if you don't want to create an enum for it and just name the values ad-hoc.

dalexeev commented 1 year ago

If we add a separate syntax for flags, then it also makes sense to add a BitField[T] metatype. It's already used in the core API (see godotengine/godot#74641).