Add 'with' and 'without' methods to EngineBitfield. by rsethc · Pull Request #1627 · godot-rust/gdext

rsethc · 2026-06-06T16:19:16Z

Add the BitAnd, BitAndAssign, and BitNot flags to bitfields for the common mask operation to remove one flag from a set of them (i.e. let flags_with_one_removed = more_flags & !excluded_flag pattern).
Allow setting a Default implementation function body, for when the #[derive(Default)] default value would not make sense. In this case, the DuplicateFlags where the Default::default() will now evaluate to the same default that is set by Node::duplicate_node and initially by Node::duplicate_node_ex.

rsethc · 2026-06-06T16:24:12Z

I am aware this needs some adjustments

possibly separate into multiple merge requests (I would think, one for the and+not trait impls, and then one for the Default codegen?)
squashing the commits in this branch into each other to eliminate 'ran lint tool' noise commits from making their way into main branch
maybe other changes like adding tests etc, haven't looked closely into that matter yet

However wanted to at least go ahead and open this for awareness.

I have a use case where I want to duplicate nodes without using instantiation, but to me the most "proper" way is to use "default" flags (without hardcoding them) and erase only the flag I don't want (USE_INSTANTIATION) with bitwise &, and passing this to duplicate_node_ex() flags(). So, the first part of that is to have the ability to perform these bitwise operations, and the second part is to also make the Default::default implementation not just zero (I assumed, from a gdext usage standpoint, that the default was indeed the = 15 shown in Godot docs... but no, it's zero, and I had to troubleshoot to find out that's why other things started breaking in my game).

GodotRust · 2026-06-06T17:33:07Z

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1627

Bromeon · 2026-06-06T18:00:59Z

Thanks for your contribution!

I have a use case where I want to duplicate nodes without using instantiation, [...] the second part is to also make the Default::default implementation not just zero (I assumed, from a gdext usage standpoint, that the default was indeed the = 15 shown in Godot docs... but no, it's zero, and I had to troubleshoot to find out that's why other things started breaking in my game).

This is a fair point, and the problem is that it's hard to define a default generally. In this case it's somewhat evident from the name DUPLICATE_DEFAULT, but what in other cases?

Here are all bitfields in Godot, some observations:

AxisFlag { X | Y | Z | ALL} has no clear default (maybe ALL, but not clear in usage)
ConnectFlags has default 0, if you look at Object::connect_flags()
ModeFlags { READ | WRITE | READ_WRITE | WRITE_READ } has no default -- FileAccess::open() needs explicit arg with at least one of those set

So, I believe the responsible thing to do here would be to not ship an automatic Default impl, and instead special-case known bitfields with a meaningful default -- which would then also be documented.

However, removing Default is a breaking change, and changing ::default() value as well (just at runtime, which is arguably worse). Any such modifications have to wait until next minor version and be announced in the migration guide.

I have a use case where I want to duplicate nodes without using instantiation, but to me the most "proper" way is to use "default" flags (without hardcoding them) and erase only the flag I don't want (USE_INSTANTIATION) with bitwise &, and passing this to duplicate_node_ex() flags().

The PR currently lacks tests -- and those would likely highlight some of the problems with ! and & operations.

For example, we have a Debug impl which pretty-prints bitfields, and this will completely break for inverted ones.
There is also the problem that inverting a bitfield will enable bits that have meaning in a future Godot version (Godot 4.7 at runtime, if extension is compiled against 4.5 API) -- which may or may not be intended. Depending on what you want to do, it might be meaningful to invert all bits, or only all known bits.

I agree that & + ! (or ~ in other languages) is common for this on a bit level, but it might also be more powerful than necessary. You could technically achieve the same with a named method:

DuplicateFlags::default() & !DuplicateFlags::USE_INSTANTIATION
DuplicateFlags::default().unset(DuplicateFlags::USE_INSTANTIATION)

So the question is, do we have other use cases for & and ! besides unsetting a bit?

Another thing I considered was a method ::all_known() or so, but I think changing Default::default (v0.6) is better.

So this definitely needs some thought. PR-wise, please also squash your commits on next update according to Contribution Guidelines.

rsethc · 2026-06-07T11:17:43Z

Thanks for taking a careful look at this! For my use case a named method would be entirely sufficient, I have no specific need for the bitwise operators themselves at the game's level. As far as defaults, an alternative -- considering the specific flags I'm generally wanting defaults for are very specifically used for particular functions (i.e. `DuplicateFlags` is for its one niche use case for passing to `Node::duplicate_node_ex().flags(...)` ) -- would perhaps be to add simply a getter in addition to the `flags` setter. Granted it might be a bit more verbose, i.e. for this, I would think to write as: ``` let duplicated = { let mut duplication = original_node.duplicate_node_ex(); let mut flags = duplication.get_flags(); flags.unset(DuplicateFlags::USE_INSTANTIATION); // if this hypothetical method returns the modified flags, wouldn't be necessary to store flags in variable. duplication.flags(flags).done() }; ``` Perhaps instead of (or in addition to, but I understand not wanting to unnecessarily proliferate the amount of ways to do the same thing for future maintenance reasons and test coverage) adding these capabilities to the flags, the builder returned by `Node::duplicate_node_ex` might directly have `unset_flag` and `set_flag` itself, which would make this the most compact. If `BitOr` weren't already "the" way to combine flags, this would cover all use cases if there were a `clear_flags` method on this builder too. ``` let duplicated = original_node.duplicate_node_ex().unset_all_flags().set_flag(the_only_flag_you_want).done(); ``` The downside with taking power away from `DuplicateFlags` is also with storing some flags, especially as a convenient `const` if one is frequently using particular flags. It would be clunky to represent that. i.e. `const MY_DUPE_FLAGS: DuplicateFlags = flag1 | flag2;` versus ``` fn apply_my_dupe_flags (mut builder: DuplicateBuilder) - { builder.set_flag(flag1); builder.set_flag(flag2); builder } ``` A consideration is consistency with existing pattern. If "the" way to work with `DuplicateFlags` so far is the bitwise operators (`|`), using methods in other cases (`&`, `!`) is potentially going to just feel strange as a user of the API. "Do they want me to use bitwise operations or not?" Ultimately I will be glad to throw together whichever solution is preferable, and clean this up (including tests).

Bromeon · 2026-06-07T19:04:45Z

@rsethc would it be possible for you to use the GitHub web interface rather than email? I'm not sure if you saw my edits, plus your message doesn't seem to apply Markdown formatting, and has overly short line lengths, making it harder to read.

Changing duplicate_node_ex API is also an option, but it would be an isolated solution that doesn't scale to the many other APIs that have the same issue. Providing unsetting on the bitfield type itself is thus more powerful.

I agree that mixing | with a named method such as unset is maybe not the most symmetric. But we already have is_set, I'm not sure if (bitfield & bit).ord() != 0 is better than bitfield.is_set(bit). For unset it's not as extreme, but still.

Maybe without is a better name than unset, as it doesn't imply changing the subject. And we could even alias | as with for symmetry. Just an idea:

let new_value = bitfield.with(MyBitfield::A).without(MyBitfield::B);

versus:

let new_value = (bitfield | MyBitfield::A) & !MyBitfield::B;

rsethc · 2026-06-09T14:00:01Z

Thanks, I'm seeing the DEFAULT enum variant of DuplicateFlags and wondering why I missed something so obvious, it's because while traveling I was still on version 0.4.5 of the godot crate, made this change offline, then began to turn it into a pull request here after a few more days. This is also the reason for the email replies (strange that GitHub does not seem to even allow Markdown when editing comment text from the web UI later on).

I will completely remove the Default stuff from this branch. It really is unnecessary given the DEFAULT variant of this enum which exists now. I'll be glad to create a separate branch+PR with the Default trait changes if you do want to bundle that into a major version (I assume) due to its breaking change nature, but in my opinion it would be equally valid to simply remove the Default implementation by no longer deriving it. Omitting it would entirely prevent the confusion I ran into, without adding as much code to maintain. [ It is now here: https://github.com/Implement the Default trait with more specific code than the derived Default, for DuplicateFlags. #1630 ]
without sounds like quite a clear name for this method to do a & ~b operation. I'll ditch the bitwise operators trait impls in favor of the without method approach.
Aliasing | i.e. BitOr with with sounds good, I will add that in this branch as well to mirror the without.
And then I will take a look at adding tests, & squash before unmarking this as Draft.

Bromeon · 2026-06-10T14:01:17Z

+    // Test that when adding a flag, where some are
+    // already present, the ones that are supposed to
+    // be added do in fact get added.
+    assert_eq!(
+        DuplicateFlags::GROUPS.with(
+            DuplicateFlags::GROUPS.with(DuplicateFlags::SIGNALS)
+        ),
+        DuplicateFlags::GROUPS.with(DuplicateFlags::SIGNALS),
+    );


I'm not sure if this is necessary -- it's kinda already tested by adding GROUPS with itself, and adding GROUPS with SIGNALS.

A more interesting case might be chaining two with calls for separate flags.

I'd also remove the comments above all assert_eq! statements; the asserts are self-explanatory. The exception is if you really want to test something specific/unexpected. But then you can also use the 3rd argument of assert_eq!.

The point of it is mainly to detect any (unlikely) weird implementation someone might replace the bitwise flags with in the future that toggles flags without checking they were already enabled. Or something else bizarre like that. Might as well.

In the spirit of might-as-well I will also add what you've mentioned below this, for both (i.e. adding with chaining test, without chaining test).

Added that test without removing the one the comment is about. Please resolve or reply depending on agree/disagree, thanks.

Bromeon · 2026-06-10T16:04:43Z

Thanks for the updates already!

Btw, don't spend time on commit messages like:

Code generator appears to emit 'a &! b' which makes clippy angry about the generated code. Adding parentheses seems to alleviate this.

Eventually this should be 1 commit, so everything else is going to be thrown away. Feel free to always squash and force-push, I'm only looking at the complete diff anyway 🙂

rsethc · 2026-06-10T16:24:57Z

Such commit messages are for the un-squashed branches in my own fork for my own notes, even though these will be lost here due to squashing in this particular branch for this PR.

Bromeon

Thanks for the update!

Bromeon · 2026-06-13T04:49:03Z

+
+/// Necessarily since `from_ord` is not `const`.
+fn no_flags() -> DuplicateFlags {
+    // Could use `Default::default()` here. Until it is broken by `!1630`, funnily enough.


Suggested change

// Could use `Default::default()` here. Until it is broken by `!1630`, funnily enough.

// Avoid `Default::default()` as its value and presence might change in a future version.

But actually I think there's not much point in having this additional indirection, just use from_ord(0) inline -- see below.

Bromeon · 2026-06-13T05:20:38Z

+const SIGNALS: DuplicateFlags = DuplicateFlags::SIGNALS;
+const GROUPS: DuplicateFlags = DuplicateFlags::GROUPS;
+const USE_INSTANTIATION: DuplicateFlags = DuplicateFlags::USE_INSTANTIATION;
+
+#[itest]
+fn bitfield_ops_with() {
+    let no_flags = no_flags();
+
+    assert_eq!(no_flags.with(USE_INSTANTIATION), USE_INSTANTIATION);
+
+    assert_eq!(
+        GROUPS.with(SIGNALS),
+        DuplicateFlags::from_ord(GROUPS.ord() | SIGNALS.ord())
+    );
+
+    assert_eq!(GROUPS.with(GROUPS), GROUPS);
+
+    assert_eq!(GROUPS.with(GROUPS.with(SIGNALS)), GROUPS.with(SIGNALS));
+
+    let with_then_with = GROUPS.with(SIGNALS).with(USE_INSTANTIATION);
+    assert!(with_then_with.is_set(GROUPS));
+    assert!(with_then_with.is_set(SIGNALS));
+    assert!(with_then_with.is_set(USE_INSTANTIATION));
+
+    assert_eq!(GROUPS.with(no_flags), GROUPS);
+}


The test pattern is inconsistent. In some cases you verify based on the numeric values, other cases with is_set (which is strictly weaker because it doesn't account for other bits being set).

Also, you can just use SCRIPTS instead of USE_INSTANTIATION so it's easier to read. And we can even go one step further to check against the numeric values -- they must be stable for compatibility.

Then next thing, you can group all operations yielding the same result, there's no point doing NO_FLAGS.with(x) check at the start and y.with(NO_FLAGS) at the end, and x being different than y 😉

It's a bit nitpicky, but I think it really helps comprehension. Here's a more compact version. I even added two checks for commutativity.

Suggested change

const SIGNALS: DuplicateFlags = DuplicateFlags::SIGNALS;

const GROUPS: DuplicateFlags = DuplicateFlags::GROUPS;

const USE_INSTANTIATION: DuplicateFlags = DuplicateFlags::USE_INSTANTIATION;

#[itest]

fn bitfield_ops_with() {

let no_flags = no_flags();

assert_eq!(no_flags.with(USE_INSTANTIATION), USE_INSTANTIATION);

assert_eq!(

GROUPS.with(SIGNALS),

DuplicateFlags::from_ord(GROUPS.ord() | SIGNALS.ord())

);

assert_eq!(GROUPS.with(GROUPS), GROUPS);

assert_eq!(GROUPS.with(GROUPS.with(SIGNALS)), GROUPS.with(SIGNALS));

let with_then_with = GROUPS.with(SIGNALS).with(USE_INSTANTIATION);

assert!(with_then_with.is_set(GROUPS));

assert!(with_then_with.is_set(SIGNALS));

assert!(with_then_with.is_set(USE_INSTANTIATION));

assert_eq!(GROUPS.with(no_flags), GROUPS);

}

const SIGNALS: DuplicateFlags = DuplicateFlags::SIGNALS; // 1

const GROUPS: DuplicateFlags = DuplicateFlags::GROUPS; // 2

const SCRIPTS: DuplicateFlags = DuplicateFlags::SCRIPTS; // 4

#[itest]

fn bitfield_ops_with() {

let no_flags = DuplicateFlags::from_ord(0);

assert_eq!(no_flags.with(GROUPS).ord(), 2);

assert_eq!(GROUPS.with(no_flags).ord(), 2);

assert_eq!(GROUPS.with(GROUPS).ord(), 2);

assert_eq!(GROUPS.with(SIGNALS).ord(), 1 | 2);

assert_eq!(GROUPS.with(GROUPS.with(SIGNALS)).ord(), 1 | 2);

assert_eq!(GROUPS.with(GROUPS).with(SIGNALS).ord(), 1 | 2);

assert_eq!(GROUPS.with(SIGNALS).with(SCRIPTS).ord(), 1 | 2 | 4);

}

Bromeon · 2026-06-13T05:20:56Z

+#[itest]
+fn bitfield_ops_without() {
+    let no_flags = no_flags();
+
+    assert_eq!(USE_INSTANTIATION.without(USE_INSTANTIATION), no_flags);
+
+    assert_eq!(SIGNALS.with(GROUPS).without(SIGNALS), GROUPS);
+
+    assert_eq!(GROUPS.without(SIGNALS), GROUPS);
+
+    assert_eq!(
+        SIGNALS
+            .with(GROUPS)
+            .without(SIGNALS.with(USE_INSTANTIATION)),
+        GROUPS
+    );
+
+    let without_then_without = GROUPS
+        .with(SIGNALS)
+        .with(USE_INSTANTIATION)
+        .without(SIGNALS)
+        .without(USE_INSTANTIATION);
+    assert!(without_then_without.is_set(GROUPS));
+    assert!(!without_then_without.is_set(SIGNALS));
+    assert!(!without_then_without.is_set(USE_INSTANTIATION));
+
+    assert_eq!(GROUPS.without(no_flags), GROUPS);
+}


Suggested change

#[itest]

fn bitfield_ops_without() {

let no_flags = no_flags();

assert_eq!(USE_INSTANTIATION.without(USE_INSTANTIATION), no_flags);

assert_eq!(SIGNALS.with(GROUPS).without(SIGNALS), GROUPS);

assert_eq!(GROUPS.without(SIGNALS), GROUPS);

assert_eq!(

SIGNALS

.with(GROUPS)

.without(SIGNALS.with(USE_INSTANTIATION)),

GROUPS

);

let without_then_without = GROUPS

.with(SIGNALS)

.with(USE_INSTANTIATION)

.without(SIGNALS)

.without(USE_INSTANTIATION);

assert!(without_then_without.is_set(GROUPS));

assert!(!without_then_without.is_set(SIGNALS));

assert!(!without_then_without.is_set(USE_INSTANTIATION));

assert_eq!(GROUPS.without(no_flags), GROUPS);

}

#[itest]

fn bitfield_ops_without() {

let no_flags = DuplicateFlags::from_ord(0);

assert_eq!(no_flags.without(no_flags).ord(), 0);

assert_eq!(GROUPS.without(GROUPS).ord(), 0);

assert_eq!(GROUPS.without(SIGNALS).ord(), 2);

assert_eq!(SIGNALS.with(GROUPS).without(SIGNALS).ord(), 2);

assert_eq!(SIGNALS.with(GROUPS).without(SIGNALS.with(SCRIPTS)).ord(), 2);

let all = GROUPS.with(SIGNALS).with(SCRIPTS);

assert_eq!(all.without(SIGNALS).ord(), 2 | 4);

assert_eq!(all.without(SIGNALS).without(SCRIPTS).ord(), 2);

}

rsethc changed the title ~~Bit fields improvements~~ Bit fields improvements: implement bitwise 'and' and 'not' operations, specify meaningful Default impl for 'DuplicateFlags'. Jun 6, 2026

rsethc force-pushed the bit-flags-operations---and-not-default branch from c5ee9ec to 3aed515 Compare June 9, 2026 14:09

rsethc mentioned this pull request Jun 9, 2026

Implement the Default trait with more specific code than the derived Default, for DuplicateFlags. #1630

Draft

3 tasks

rsethc changed the title ~~Bit fields improvements: implement bitwise 'and' and 'not' operations, specify meaningful Default impl for 'DuplicateFlags'.~~ Implement bitwise 'and' and 'not' operations for DuplicateFlags, to un-set particular flag. Jun 9, 2026

Bromeon added feature Adds functionality to the library c: engine Godot classes (nodes, resources, ...) labels Jun 9, 2026

Bromeon added this to the 0.5.x milestone Jun 9, 2026

rsethc force-pushed the bit-flags-operations---and-not-default branch from 3aed515 to 35a0d9a Compare June 9, 2026 14:28

Bromeon reviewed Jun 10, 2026

View reviewed changes

rsethc force-pushed the bit-flags-operations---and-not-default branch from ebe0c98 to 370ac19 Compare June 10, 2026 16:46

rsethc changed the title ~~Implement bitwise 'and' and 'not' operations for DuplicateFlags, to un-set particular flag.~~ Add 'with' and 'without' methods to EngineBitfield. Jun 10, 2026

rsethc force-pushed the bit-flags-operations---and-not-default branch from 370ac19 to 27d90ca Compare June 10, 2026 16:47

rsethc marked this pull request as ready for review June 10, 2026 16:48

rsethc force-pushed the bit-flags-operations---and-not-default branch from 27d90ca to 8ebf527 Compare June 10, 2026 16:48

rsethc marked this pull request as draft June 10, 2026 16:55

Add 'with' and 'without' methods to EngineBitfield.

8709aff

rsethc force-pushed the bit-flags-operations---and-not-default branch from 499dfba to 8709aff Compare June 10, 2026 17:04

rsethc marked this pull request as ready for review June 10, 2026 17:04

Bromeon reviewed Jun 13, 2026

View reviewed changes

	// Could use `Default::default()` here. Until it is broken by `!1630`, funnily enough.
	// Avoid `Default::default()` as its value and presence might change in a future version.

Uh oh!

Conversation

rsethc commented Jun 6, 2026

Uh oh!

rsethc commented Jun 6, 2026

Uh oh!

GodotRust commented Jun 6, 2026

Uh oh!

Bromeon commented Jun 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rsethc commented Jun 7, 2026 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bromeon commented Jun 7, 2026

Uh oh!

rsethc commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Bromeon Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

rsethc Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

rsethc Jun 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Bromeon commented Jun 10, 2026

Uh oh!

rsethc commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Bromeon left a comment

Choose a reason for hiding this comment

Uh oh!

Bromeon Jun 13, 2026

Choose a reason for hiding this comment

Uh oh!

Bromeon Jun 13, 2026

Choose a reason for hiding this comment

Uh oh!

Bromeon Jun 13, 2026

Choose a reason for hiding this comment

Uh oh!

Bromeon Jun 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Bromeon commented Jun 6, 2026 •

edited

Loading

rsethc commented Jun 7, 2026 via email •

edited

Loading

rsethc commented Jun 9, 2026 •

edited

Loading

rsethc commented Jun 10, 2026 •

edited

Loading