Simplify module structure II

[Bromeon]

[Edit] Results are now visible on https://godot-rust.github.io/docs.

---

This PR is a continuation of #788 and addresses the remaining, somewhat ambitious tasks for module cleanup.

Goals:

• Every symbol appears at most once in prelude
• Every symbol appears at exactly once outside prelude
• 2-3 modules (inside the crate) are the maximum nesting depth
• Modules named according to related functionality from user point of view

Changes:

1. nativescript module

   • Rename to export. Rationale: "native script" is quite a wide term, while most the module's current functionality is directly related to exporting symbols from Rust. In practice, godot-rust is always (?) used as a native script. Other potential use cases, as a pure Godot API library or with native_calls, seem to be the exception, if at all.
   • Along with renaming, the nativescript feature is removed.
   • Nested symbols in export::{properties, methods} are moved one level up.
   • As a result, we can avoid very long qualifiers and multiple candidates for use statements.
     • nativescript::init::property::hint::EnumHint -> export::hint::EnumHint
     • nativescript::export::method::MethodBuilder -> export::MethodBuilder

2. api module

   • Remove inner types like api::area::Area, as they are already present in api.
   • Remove all modules which would then become empty.
   • Create doc links between class (Camera2D) and related module (camera_2d).

3. Smaller top-level modules

   • Add init (previously part of nativescript).
   • Add profiler (previously part of nativescript).
   • Extend log with related macros.
   • Remove macros and distribute its symbols to the most fitting API.

4. prelude module

   • Remove a few macros (godot_gdnative_init etc.) from the prelude, suggesting that godot_init should be used.
   • user_data symbols are accessible through prelude::user_data instead of prelude directly. This mirrors common usage in examples.

5. core_types module
   No changes in this PR; see PR discussion for reasons and potential alternatives.

[Bromeon]

Planned changes in this PR:

• A few renamings, since relation implied by old module structure no longer exists

  • Example: property::ExportInfo -> PropertyInfo or so
  • ArgumentError[Kind] -> ArgError[Kind]? (consistent with ArgBuilder)

• A module gdnative::global, gdnative::runtime or similar, which combines functionality like:

  • InitHandle, InitializeInfo, TerminateInfo, godot_nativescript_init, ...
  • Macros like godot_print, godot_error, ...
  • Logging (currently in log namespace)

• Possibly rename core_types?

  • builtin, builtins -- official GDScript name is built-in types, however there are also symbols like ToVariant etc.
  • value was suggested; in my eyes misleading as it's not restricted to value types and this is a very general term ("value" can also just mean expression value)

• Find candidates in prelude for removal or deprecation

• See if there's any need in api to address duplicate symbols, such as api::Camera2D vs. api::camera_2d::Camera2D (see also my next reply).

Eventually I will squash some related commits, but possibly not all, as it's quite a big change.
One commit will be fully removed, its only purpose is that CI runs on my own fork on every push.

[chitoyuu]

Overall I think it's a good idea to get rid of the deep nested import paths. I don't have any objections removing the nativescript feature -- it almost never made sense to disable it in the first place. A few observations on the changes so far:

• The path export::profiler doesn't make much sense. The profiler module can probably be moved to the top level once the nativescript feature is removed.
• Instance and RefInstance (or TInstance, whatever you might want to rename it to) are not related to exporting. They are more similar to Ref / TRef than other types in the export module, and should probably be moved alongside them once the nativescript feature is removed.

Regarding the planned changes:

• Example: property::ExportInfo -> PropertyInfo or so

This feels very confusing to me. The ExportInfo type does not contain all the information about a property, only a very small part of it about how to export the value: its Variant type and editor hints.

A module gdnative::global, gdnative::runtime or similar, which combines functionality like:

• InitHandle, InitializeInfo, TerminateInfo, godot_nativescript_init, ...
• Macros like godot_print, godot_error, ...
• Logging (currently in log namespace)

Isn't this contrary to the effort of flattening nested modules? I think it's necessary to set the priorities clear for the task: do we want less symbols at each level, or a shallower hierarchy? These goals defeat each other. They cannot be achieved at the same time.

Even with the current changes, I wouldn't say the top-level namespace is cluttered enough for this to be necessary.

Possibly rename core_types?

• builtin, builtins -- official GDScript name is built-in types, however there are also symbols like ToVariant etc.

I share the sentiment that there are shortcomings to the core_types name, but I doubt it's optimal to rename it to builtin or builtins in particular:

• While the types are built-in to GDScript, they aren't in any sense "built-in" to Rust.
• Using the same terminology as GDScript can make teaching easier, but only if the "built-in" classification is internalized in the first place. Otherwise, it's just yet another name to learn by heart.
• "Built-in" still feels like two words even with the hyphen/underscore removed, and isn't much shorter than "core_types".

• See if there's any need in api or we leave it as-is

Unless you want the top-level to be polluted by a bazillion generated types like it was in 0.8.x, api absolutely needs to stay.

[Bromeon]

Thanks a lot for your valuable feedback! 👍🏼

• The path export::profiler doesn't make much sense. The profiler module can probably be moved to the top level once the nativescript feature is removed.

• Instance and RefInstance (or TInstance, whatever you might want to rename it to) are not related to exporting. They are more similar to Ref / TRef than other types in the export module, and should probably be moved alongside them once the nativescript feature is removed.

Agree to both. We could put Instance/TInstance into object module.
Regarding profiler, I wonder if it can be somehow grouped next to the log/print/init functionality, see below.

---

• Example: property::ExportInfo -> PropertyInfo or so

This feels very confusing to me. The ExportInfo type does not contain all the information about a property, only a very small part of it about how to export the value: its Variant type and editor hints.

Ok, maybe I misunderstood the purpose, since it was explicitly in the property module before.
Would you leave it at gdnative::export::ExportInfo?

---

A module gdnative::global, gdnative::runtime or similar [...]

Isn't this contrary to the effort of flattening nested modules? I think it's necessary to set the priorities clear for the task: do we want less symbols at each level, or a shallower hierarchy? These goals defeat each other. They cannot be achieved at the same time.

Even with the current changes, I wouldn't say the top-level namespace is cluttered enough for this to be necessary.

I don't see it as a single-dimensional optimization problem where we need to minimize nesting, or number of symbols per level. Important is that the grouping and organization is somewhat intuitive for the user, and that they find functionality they are looking for. Of course, that's very subjective and not easily quantifiable -- that's why we can't just automate these changes 🙂

In #788 I created a macros module, but I think it's not ideal. First, it groups symbols by "kind" (being a macro) rather than functionality, second it mixes unrelated topics: initialization, printing/debugging, and class registration (godot_wrap_method).

It looks like with the renaming of export, we can isolate the following functionalities:

• Initialization (InitHandle, InitializeInfo, TerminateInfo, godot_nativescript_init, ...)
• Profiling
• Logging
• Printing/Debugging

So we have two options: create a top-level module for each of those, or try to somehow group them into a global (or similar) module. The first approach reduces nesting, the second makes the other modules like export, api, object stand out more, as they're arguably more important. At least profiling/logging is rather advanced functionality.

Could of course also be a mixed approach like:

//! top-level lib.rs

mod global {
   ... // print + debug macros
   ... // entire init related functionality

   // more advanced functionality in their own modules
   mod log { ... }
   mod profiler { ... }
}

I don't have a very strong opinion here, maybe someone else can also give their input?

---

• Using the same terminology as GDScript can make teaching easier, but only if the "built-in" classification is internalized in the first place. Otherwise, it's just yet another name to learn by heart.

True, but unlike "core types" they are based in Godot's documentation and not chosen arbitrarily.

• "Built-in" still feels like two words even with the hyphen/underscore removed, and isn't much shorter than "core_types".

This wasn't an obstacle for nativescript 😉
No, I agree. I've seen "builtins" used in tech lingo, but that doesn't make it correct.

Alternatives would be: "Simple types", "basic types", "primitives". More?

---

• See if there's any need in api or we leave it as-is

Unless you want the top-level to be polluted by a bazillion generated types like it was in 0.8.x, api absolutely needs to stay.

It definitely stays. What I meant is, if we should do something about duplicated symbols like api::camera_2d::Camera2D vs. api::Camera2D. The camera_2d module simply exists to emulate nested types; in C++ we would have

class Camera2D {
public:
    enum AnchorMode { ... };
    enum Camera2DProcessMode { ... };
};

which then becomes in Rust:

pub mod camera_2d {
    pub struct Camera2D { ... }
    pub struct AnchorMode { ... }
    pub struct Camera2DProcessMode { ... }
}
pub use camera_2d::Camera2D;

But again, it's a low-prio thing, would just be interesting to hear other people's opinions.

[chitoyuu]

Would you leave it at gdnative::export::ExportInfo?

Unless you rename the Export trait together with it I suppose? It is specifically for properties, but it doesn't cover everything about properties. The Export name is originally chosen because the GDScript keyword is export.

grouping and organization is somewhat intuitive for the user, and that they find functionality they are looking for

I don't think I have seen many crates with a global module. Most seem to be content with leaving them at the top level (under functionality-specific submodules when applicable). Of course each library is different from each other, but I doubt it'll be intuitive if there aren't many examples of projects doing so already. Do you have some examples crates that have "global" modules under the top-level?

I created a macros module, but I think it's not ideal. First, it groups symbols by "kind" (being a macro) rather than functionality, second it mixes unrelated topics ... So we have two options: create a top-level module for each of those, or try to somehow group them into a global (or similar) module.

I agree that the macros module approach isn't quite ideal. Personally I think the first approach is more intuitive here -- see the section above -- but I could be wrong.

It definitely stays. What I meant is, if we should do something about duplicated symbols like api::camera_2d::Camera2D vs. api::Camera2D. The camera_2d module simply exists to emulate nested types

Thanks for the clarification. Perhaps you can edit #811 (comment) to include this?

---

This wasn't an obstacle for nativescript ...

This reminds me: while we're at it, what do you think about renaming the NativeClass trait to NativeScript? The former one was invented here, and there has been incidents where new users confused it with GodotObject, interpreting "NativeClass" as "native (Godot API) class".

One potential issue is collision with api::NativeClass the resource type, but hopefully that can be dealt with some documentation, since the use frequency of api::NativeClass is probably rather low anyway?

[Bromeon]

Would you leave it at gdnative::export::ExportInfo?

Unless you rename the Export trait together with it I suppose? It is specifically for properties, but it doesn't cover everything about properties. The Export name is originally chosen because the GDScript keyword is export.

Makes sense 👍🏼

---

I don't think I have seen many crates with a global module. Most seem to be content with leaving them at the top level (under functionality-specific submodules when applicable). Of course each library is different from each other, but I doubt it'll be intuitive if there aren't many examples of projects doing so already. Do you have some examples crates that have "global" modules under the top-level?

Yes, but the name varies. For "ubiquitious" functionality, core is quite common. For things used here and there, "utils" or "misc" or so are typical module names.

• bevy has utils, core but also specific log etc.
• salva2d has helper
• rapier2d has utils
• egui has util, but they also have many identifiers on top level
• rg3d has utils, but a handful on top level as well

So everyone does it slightly differently. It seems to be that crates of larger scope (e.g. game or physics engines) tend to have fewer symbols directly on the top level.

---

Thanks for the clarification. Perhaps you can edit #811 (comment) to include this?

Done 👍🏼

---

This reminds me: while we're at it, what do you think about renaming the NativeClass trait to NativeScript? The former one was invented here, and there has been incidents where new users confused it with GodotObject, interpreting "NativeClass" as "native (Godot API) class".

One potential issue is collision with api::NativeClass the resource type, but hopefully that can be dealt with some documentation, since the use frequency of api::NativeClass is probably rather low anyway?

I assume you mean it collides with api::NativeScript.

Generally we can do it, I would probably postpone it to #773 however (still v0.10.0), where we can also discuss removal/deprecation strategy. One thing to consider is how this concept is called with Godot 4 and GDExtension -- just so we don't need to rename it immediately again.

[chitoyuu]

Yes, but the name varies. For "ubiquitious" functionality, core is quite common. For things used here and there, "utils" or "misc" or so are typical module names.

• bevy has utils, core but also specific log etc.
• salva2d has helper
• rapier2d has utils
• egui has util, but they also have many identifiers on top level
• rg3d has utils, but a handful on top level as well

I have checked all these crates:

• bevy's utils consists mostly re-exports of data structures defined outside the engine.
• salva2d's helper contains a sole filter_from_mask function that does not seem to pertain to the purpose of the library.
• rapier2d's utils contains a few extension traits that only marginally relate to what the library is doing.
• egui's util contains miscellaneous data structures.
• rg3d's utils is the only one that contains a substantial collection of functions, types and algorithms. However, the items within does not seem to resemble what you're trying to group into a "global" module here.

Overall, I'd consider those examples very tangential. bevy is, in particular, a terrible one for this argument, given how many modules it has on the top level despite having a utils module: it's not nearly as much of a "global" module as you'd like to make it out to be.

I assume you mean it collides with api::NativeScript.

Yes. Sorry for the confusion.

Generally we can do it, I would probably postpone it to #773 however (still v0.10.0), where we can also discuss removal/deprecation strategy. One thing to consider is how this concept is called with Godot 4 and GDExtension -- just so we don't need to rename it immediately again.

Sure, it can be postponed, but I wonder if there is any reason to consider Godot 4 naming, if GDExtension will be a divergent fork anyway? Wouldn't it be confusing to use Godot 4 names in a branch that will only support Godot 3.x?

[Bromeon]

Overall, I'd consider those examples very tangential. bevy is, in particular, a terrible one for this argument, given how many modules it has on the top level despite having a utils module: it's not nearly as much of a "global" module as you'd like to make it out to be.

In general "others do it, so we should do it too" is not a particularly good argument, unless it's really industry-wide practice or we understand the reason why they're doing it. So I'm not trying to "prove" anything here, was merely trying to answer your question if I've seen this before 🙂

What I think is more relevant to decide this is:

• Do we want the user to need fewer nestings in general, i.e. shorter use statements?
• Or do we want fewer nestings for often-used functionality, but accept to have more advanced/specific functionality behind 2 levels (e.g. logging and profiling, and possibly more integrations/tools in the future)?

The way we design implicitly sends a message on how important/accessible each symbol in the API is. Having more modules on the top level means the user probably devotes less attention to each. On the other hand, he directly sees profile/log information, which can be harder to discover otherwise. Ultimately it's a trade-off, but I agree it's a small one, and as you say we're not yet at a level where the top level is cluttered. So we can also start with this, and reorganize again if it becomes a problem.

---

Sure, it can be postponed, but I wonder if there is any reason to consider Godot 4 naming, if GDExtension will be a divergent fork anyway? Wouldn't it be confusing to use Godot 4 names in a branch that will only support Godot 3.x?

I didn't mean to use Godot 4 names that have no meaning in Godot 3. Rather, if e.g. Godot 4 uses "native class" as the official term, so we now change it and break users code, only to break it again when they migrate to Godot 4.

So, my thinking was more: "if we know a name that makes sense now and will still make sense in Godot 4, we might as well use that". Not a reason against NativeScript in particular.

Are there other potential candidates here? We usually talk about "exported methods/properties", does it make sense to extend this notion to classes/structs?

[jacobsky]

I think this is a good idea. I think I showed up a bit late as I don't have anything new to add that chitoyuu hadn't already mentioned.

Regarding core_types, it seems like the godot-cpp just files them all under src/variant, I don't have a strong opinion on renaming it so long as the meaning is clear that these are Godot types that have an implementation in pure-rust.

[chitoyuu]

The way we design implicitly sends a message on how important/accessible each symbol in the API is. Having more modules on the top level means the user probably devotes less attention to each. On the other hand, he directly sees profile/log information, which can be harder to discover otherwise. Ultimately it's a trade-off, but I agree it's a small one, and as you say we're not yet at a level where the top level is cluttered. So we can also start with this, and reorganize again if it becomes a problem.

I agree that keeping users' focus on the important stuff is an important aspect of API organization. When adding a new level of nesting, though, perhaps it's also worth it to consider the perspective of organic growth over time. For a Rust library, new modules are cheap to add, but existing ones are expensive to re-organize: this PR and #788 being the prime examples of this. It was organic growth that led us down this path of super-nested modules, and it will again if we don't start to artificially "price" new public modules at a higher point.

Ultimately, we're still in 0.x with future breaking changes in the plans, so I concur that it isn't that big a problem even if we make some mistakes.

I didn't mean to use Godot 4 names that have no meaning in Godot 3. Rather, if e.g. Godot 4 uses "native class" as the official term, so we now change it and break users code, only to break it again when they migrate to Godot 4.

So, my thinking was more: "if we know a name that makes sense now and will still make sense in Godot 4, we might as well use that". Not a reason against NativeScript in particular.

Ah, that makes sense! 👍

[Waridley]

One argument in favor of core_types is that they are under core in the engine's source code: https://github.com/godotengine/godot/tree/3.x/core
However, they were apparently moved to core/variant in 4.0: https://github.com/godotengine/godot/tree/master/core/variant

I definitely think initialization-related symbols should be in their own module. They're usually only ever used in one file in a project. In mine, I use them in every some_module.rs file (equivalent of mod.rs but adjacent to the folder instead of inside it). The vast majority of the time, I don't need to think about InitHandle, etc.

Logging and profiling seem related-enough to printing and debugging to be together IMO... Although printing and debugging are so common that it could make a lot of sense to put them in the root of the crate, but I'm not sure the same could be said for logging/profiling.

• See if there's any need in api to address duplicate symbols, such as api::Camera2D vs. api::camera_2d::Camera2D (see also my next reply).

Does the parent class actually need to be defined in the module at all? Is this just due to how the bindings generator works, or is there some other reason for it? Could it just be:

pub mod camera_2d {
    pub struct AnchorMode { ... }
    pub struct Camera2DProcessMode { ... }
}
pub struct Camera2D { ... }

One other related thought that hasn't been brought up yet: Is there any way we could make the global constants available in a module, instead of associated with the GlobalConstants struct? I know that's how they're exposed in the API, but needing to use GlobalConstants::KEY_ESCAPE etc. everywhere they're used instead of being able to import the constants directly is annoying.

[Bromeon]

Thanks for your input as well, Waridley!

Regarding core_types, yes, variant would be an option. Should just not be confused with Variant itself.
Maybe variants, although we also don't pluralize object and others...

Does the parent class actually need to be defined in the module at all? Is this just due to how the bindings generator works, or is there some other reason for it? Could it just be:

pub mod camera_2d {
    pub struct AnchorMode { ... }
    pub struct Camera2DProcessMode { ... }
}
pub struct Camera2D { ... }

Would also be possible. The slight disadvantage is that one has to import 2 different modules when dealing with 1 specific class, but I'd say that's OK. It should still be clear which snake_case module belongs to which PascalCase struct.

---

One other related thought that hasn't been brought up yet: Is there any way we could make the global constants available in a module, instead of associated with the GlobalConstants struct? I know that's how they're exposed in the API, but needing to use GlobalConstants::KEY_ESCAPE etc. everywhere they're used instead of being able to import the constants directly is annoying.

Are these the same symbols as documented in @GlobalScope?

Yes, I think this should be doable, and would allow importing instead of qualifying the struct. Will need changes in the code generator, so probably out of scope for this refactoring. Could you open a separate issue for it?

[Waridley]

Would also be possible. The slight disadvantage is that one has to import 2 different modules when dealing with 1 specific class, but I'd say that's OK. It should still be clear which snake_case module belongs to which PascalCase struct.

That's true. Personally I'd like this trade-off, but I'd definitely understand if not everyone else liked it. I won't complain either way 🙂

I don't think I have any other feedback; I like what you're doing! My imports are a mess right now 😆

[Bromeon]

Implemented most of the changes, will do the rest later today or tomorrow.
Thanks for all the input so far!

Pending / to be discussed:

• rename core_types to primitives, variant or something?
• remove nested duplicate symbols in api (see Waridley proposal)
• decide what to do with godot_print etc.
• ...?

[Bromeon]

bors try

[bors]

try

Build succeeded:

• full-ci

[Bromeon]

Edited initial post to give an up-to-date overview of the changes.

[Bromeon]

bors try

[bors]

try

Build succeeded:

• full-ci

[jacobsky]

Sorry for the late comment! I agree we can go ahead and merge this in. I think the renaming is something we can decide on later (perhaps by vote).

[Bromeon]

bors r+

[bors]

Canceled.

[Bromeon]

Added commit to fix warning with test_transform2d_behavior_impl(), which was unconditionally compiled but only conditionally used.

bors r+

[bors]

Build succeeded:

• full-ci