Skip to content

Commit 1add2d0

Browse files
authored
Merge pull request #1099 from godot-rust/qol/reduce-minimal-classes
Reduce number of classes in minimal codegen
2 parents 78ca889 + 43b7ada commit 1add2d0

20 files changed

+501
-374
lines changed

.github/composite/godot-itest/action.yml

+2-1
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,10 @@ runs:
142142
targetArgs="--target $TARGET"
143143
fi
144144
145+
# Keep `--no-default-features` even if it's currently redundant. Features may change.
145146
cargo build -p itest --no-default-features ${{ inputs.rust-extra-args }} $targetArgs
146147
147-
# Instead of modifying .gdextension, rename the output directory
148+
# Instead of modifying .gdextension, rename the output directory.
148149
if [[ -n "$TARGET" ]]; then
149150
rm -rf target/debug
150151
mv target/$TARGET/debug target

.github/workflows/full-ci.yml

+1
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ jobs:
332332
os: ubuntu-22.04
333333
artifact-name: linux-nightly
334334
godot-binary: godot.linuxbsd.editor.dev.x86_64
335+
# Important to keep both experimental-threads and codegen-full. Some itests (native_st_audio) require both.
335336
rust-extra-args: --features itest/experimental-threads,itest/codegen-full-experimental,godot/api-custom,godot/serde,itest/register-docs
336337

337338
- name: linux-release

godot-codegen/src/context.rs

+6-2
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,12 @@ impl<'a> Context<'a> {
114114
break;
115115
}
116116
}
117-
let (nearest_index, nearest_enum_name) =
118-
nearest.expect("at least one base must have notifications");
117+
let (nearest_index, nearest_enum_name) = nearest.unwrap_or_else(|| {
118+
panic!(
119+
"class {}: at least one base must have notifications; possibly, its direct base has been removed from minimal codegen",
120+
class_name.godot_ty
121+
)
122+
});
119123

120124
// For all bases inheriting most-derived base that has notification constants, reuse the type name.
121125
for i in (0..nearest_index).rev() {

godot-codegen/src/generator/native_structures.rs

+10-4
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::context::Context;
99
use crate::generator::builtins;
1010
use crate::models::domain::{ExtensionApi, ModName, NativeStructure, TyName};
1111
use crate::util::ident;
12-
use crate::{conv, util, SubmitFn};
12+
use crate::{conv, special_cases, util, SubmitFn};
1313
use proc_macro2::TokenStream;
1414
use quote::{format_ident, quote};
1515
use std::path::Path;
@@ -25,6 +25,11 @@ pub fn generate_native_structures_files(
2525

2626
let mut modules = vec![];
2727
for native_structure in api.native_structures.iter() {
28+
// Some may be excluded in minimal codegen, because they hold codegen-excluded classes as fields.
29+
if special_cases::is_native_struct_excluded(&native_structure.name) {
30+
continue;
31+
}
32+
2833
let module_name = ModName::from_godot(&native_structure.name);
2934
let class_name = TyName::from_godot(&native_structure.name);
3035

@@ -232,10 +237,11 @@ fn make_native_structure_field_and_accessor(
232237
(field_def, accessor)
233238
}
234239

240+
/// Native structures use a different format for enums than the rest of the JSON file.
241+
/// If we detect a scoped field, convert it to the enum format expected by to_rust_type().
242+
///
243+
/// Example: `TextServer::Direction` -> `enum::TextServer.Direction`.
235244
fn normalize_native_structure_field_type(field_type: &str) -> String {
236-
// native_structures uses a different format for enums than the
237-
// rest of the JSON file. If we detect a scoped field, convert it
238-
// to the enum format expected by to_rust_type.
239245
if field_type.contains("::") {
240246
let with_dot = field_type.replace("::", ".");
241247
format!("enum::{}", with_dot)

godot-codegen/src/special_cases/codegen_special_cases.rs

+52-49
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,16 @@ pub(crate) fn is_class_excluded(_godot_class_name: &str) -> bool {
3434
false
3535
}
3636

37+
#[cfg(not(feature = "codegen-full"))]
38+
pub(crate) fn is_native_struct_excluded(native_struct: &str) -> bool {
39+
native_struct == "CaretInfo"
40+
}
41+
42+
#[cfg(feature = "codegen-full")]
43+
pub(crate) fn is_native_struct_excluded(_native_struct: &str) -> bool {
44+
false
45+
}
46+
3747
#[cfg(not(feature = "codegen-full"))]
3848
fn is_type_excluded(ty: &str, ctx: &mut Context) -> bool {
3949
use crate::conv;
@@ -122,69 +132,62 @@ pub(crate) fn is_utility_function_excluded(
122132
// Classes for minimal config
123133
#[cfg(not(feature = "codegen-full"))]
124134
const SELECTED_CLASSES: &[&str] = &[
125-
"AnimatedSprite2D",
126-
"Area2D",
127-
"ArrayMesh",
128-
"AudioStreamPlayer",
129-
"BaseButton",
130-
"BoxMesh",
131-
"Button",
132-
"Camera2D",
133-
"Camera3D",
134-
"CanvasItem",
135-
"CanvasLayer",
136-
"ClassDB",
137-
"CollisionObject2D",
138-
"CollisionShape2D",
139-
"Control",
140-
"EditorPlugin",
141-
"EditorExportPlugin",
142-
"Engine",
143-
"FileAccess",
144-
"GDScript",
145-
"HTTPRequest",
146-
"Image",
147-
"ImageTextureLayered",
148-
"Input",
149-
"InputEvent",
150-
"InputEventAction",
151-
"Label",
152-
"MainLoop",
153-
"Marker2D",
154-
"Mesh",
135+
// Core class hierarchy
136+
"Object",
155137
"Node",
138+
"CanvasItem", // base of Node2D
156139
"Node2D",
157140
"Node3D",
158-
"Node3DGizmo",
159-
"Object",
160-
"OS",
161-
"PackedScene",
162-
"PathFollow2D",
163-
"PhysicsBody2D",
164-
"PrimitiveMesh",
165141
"RefCounted",
166-
"RenderingServer",
167142
"Resource",
168-
"ResourceFormatLoader",
143+
//
144+
// Runtime + reflection support
145+
"ClassDB",
146+
"Engine",
147+
"OS",
148+
//
149+
// Editor plugins
150+
"EditorPlugin",
151+
"EditorExportPlugin",
152+
//
153+
// I/O and save/load
169154
"ResourceLoader",
170155
"ResourceSaver",
171-
"RigidBody2D",
156+
"FileAccess",
157+
//
158+
// Scene (node_test, rpc_test)
159+
"MainLoop", // base of SceneTree
172160
"SceneTree",
173-
"SceneTreeTimer",
161+
//
162+
// Script instances
174163
"Script",
175164
"ScriptExtension",
176165
"ScriptNameCasing",
177166
"ScriptLanguage",
178167
"ScriptLanguageExtension",
179-
"Sprite2D",
180-
"SpriteFrames",
181-
"TextServer",
182-
"TextServerExtension",
168+
"GDScript",
169+
//
170+
// Example resources
171+
"PackedScene", // manual_extensions
183172
"Texture",
184-
"Texture2DArray",
185-
"TextureLayered",
186-
"Time",
187-
"Timer",
173+
//
174+
// Meshes (virtual_methods_test)
175+
"Mesh",
176+
"ArrayMesh", // enum_test, 1 case, but small API
177+
"PrimitiveMesh",
178+
//
179+
// Windowing + Input (virtual_methods_test)
188180
"Viewport",
189181
"Window",
182+
"Input",
183+
"InputEvent",
184+
"InputEventAction",
185+
//
186+
// Godot servers (for RID support)
187+
"RenderingServer",
188+
//
189+
// Misc
190+
"Time", // usage: enum_test.enum_hash()
191+
"HTTPRequest",
192+
"ResourceFormatLoader", // TODO: replace?
190193
];

godot-codegen/src/special_cases/special_cases.rs

+5
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,11 @@ pub fn is_class_deleted(class_name: &TyName) -> bool {
8989
|| is_godot_type_deleted(&class_name.godot_ty)
9090
}
9191

92+
/// Native-struct types excluded in minimal codegen, because they hold codegen-excluded classes as fields.
93+
pub fn is_native_struct_excluded(ty: &str) -> bool {
94+
codegen_special_cases::is_native_struct_excluded(ty)
95+
}
96+
9297
pub fn is_godot_type_deleted(godot_ty: &str) -> bool {
9398
// Note: parameter can be a class or builtin name, but also something like "enum::AESContext.Mode".
9499

godot/src/prelude.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ pub use super::meta::error::{ConvertError, IoError};
1616
pub use super::meta::{FromGodot, GodotConvert, ToGodot};
1717

1818
pub use super::classes::{
19-
AudioStreamPlayer, Camera2D, Camera3D, IAudioStreamPlayer, ICamera2D, ICamera3D, INode,
20-
INode2D, INode3D, IObject, IPackedScene, IRefCounted, IResource, ISceneTree, Input, Node,
19+
INode, INode2D, INode3D, IObject, IPackedScene, IRefCounted, IResource, ISceneTree, Node,
2120
Node2D, Node3D, Object, PackedScene, RefCounted, Resource, SceneTree,
2221
};
2322
pub use super::global::{

itest/godot/SpecialTests.gd

+6-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,12 @@ extends TestSuiteSpecial
1818
# testing this at the moment, since we don't have any way to let frames pass in between the start and end of
1919
# an integration test.
2020
func test_collision_object_2d_input_event():
21-
var collision_object := CollisionObject2DTest.new()
21+
if not IntegrationTests.is_full_codegen():
22+
print("Skip in minimal codegen: test_collision_object_2d_input_event")
23+
return
24+
25+
# Dynamic instantiation, to still parse if Rust class is disabled.
26+
var collision_object: Variant = ClassDB.instantiate("CollisionObject2DTest")
2227
collision_object.input_pickable = true
2328

2429
var collision_shape := CollisionShape2D.new()

itest/rust/src/builtin_tests/containers/array_test.rs

+5-3
Original file line numberDiff line numberDiff line change
@@ -399,9 +399,11 @@ fn untyped_array_return_from_godot_func() {
399399
assert_eq!(result, varray![child, Variant::nil(), NodePath::default()]);
400400
}
401401

402-
// TODO All API functions that take a `Array` are even more obscure and not included in
403-
// `SELECTED_CLASSES`. Decide if this test is worth having `Texture2DArray` and `Image` and their
404-
// ancestors in the list.
402+
// Conditional, so we don't need Texture2DArray > ImageTextureLayered > TextureLayered > Texture in minimal codegen.
403+
// Potential alternatives (search for "typedarray::" in extension_api.json):
404+
// - ClassDB::class_get_signal_list() -> Array<Dictionary>
405+
// - Compositor::set_compositor_effects( Array<Gd<Compositor>> )
406+
#[cfg(feature = "codegen-full-experimental")]
405407
#[itest]
406408
fn typed_array_pass_to_godot_func() {
407409
use godot::classes::image::Format;

itest/rust/src/builtin_tests/containers/callable_test.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -586,8 +586,8 @@ pub mod custom_callable {
586586

587587
let eq = match GdextBuild::godot_runtime_version_triple() {
588588
(4, 1..=3, _) => 1,
589-
(4, 4, 0) => 2, // changed in https://github.com/godotengine/godot/pull/96797.
590-
_ => 1, // changed in https://github.com/godotengine/godot/pull/103647.
589+
(4, 4, 0..=1) => 2, // changed in https://github.com/godotengine/godot/pull/96797.
590+
_ => 1, // changed in https://github.com/godotengine/godot/pull/103647.
591591
};
592592

593593
assert_eq!(eq_count(&at), eq, "hash collision, eq for a needed");

itest/rust/src/engine_tests/codegen_test.rs

+3-2
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
use crate::framework::itest;
1212
use godot::builtin::inner::InnerColor;
13-
use godot::classes::{FileAccess, HttpRequest, IHttpRequest, Image};
13+
use godot::classes::{FileAccess, HttpRequest, IHttpRequest, RenderingServer};
1414
use godot::prelude::*;
1515

1616
#[itest]
@@ -53,7 +53,8 @@ fn codegen_static_class_method() {
5353

5454
#[itest]
5555
fn codegen_constants() {
56-
assert_eq!(Image::MAX_WIDTH, 16777216);
56+
assert_eq!(RenderingServer::CANVAS_ITEM_Z_MIN, -4096);
57+
//assert_eq!(Image::MAX_WIDTH, 16777216);
5758
// assert_eq!(Material::RENDER_PRIORITY_MIN, -128);
5859
}
5960

itest/rust/src/engine_tests/mod.rs

+2-4
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,8 @@ mod codegen_enums_test;
1111
mod codegen_test;
1212
mod engine_enum_test;
1313
mod gfile_test;
14-
/// Native audio structure tests are only enabled when both the `experimental-threads` and `codegen-full` features are active. The tests
15-
/// require these features to be able to execute.
16-
#[cfg(all(feature = "experimental-threads", feature = "codegen-full"))]
17-
mod native_audio_structures_test;
14+
mod native_st_niche_audio_test;
15+
mod native_st_niche_pointer_test;
1816
mod native_structures_test;
1917
mod node_test;
2018
mod save_load_test;

itest/rust/src/engine_tests/native_audio_structures_test.rs renamed to itest/rust/src/engine_tests/native_st_niche_audio_test.rs

+4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
66
*/
77

8+
// Requires BOTH full codegen and experimental-threads. The latter would compile without, but crash at runtime.
9+
// More tests on native structures are in native_structure_full_codegen_tests.rs.
10+
#![cfg(all(feature = "codegen-full", feature = "experimental-threads"))]
11+
812
use std::thread;
913
use std::time::Duration;
1014

0 commit comments

Comments
 (0)