From 7f81cc8ea0615308b967e5e616a02e51ee36e305 Mon Sep 17 00:00:00 2001 From: Logan Spencer Date: Tue, 30 Dec 2025 08:55:05 -0600 Subject: [PATCH 1/5] Physics: Add step handling to CharacterBody3D Adds automatic stair stepping using up-forward-down trace algorithm. Includes visual position smoothing for camera integration. New properties: step_enabled, step_height, step_smooth_enabled, step_smooth_speed New method: get_visual_position() --- doc/classes/CharacterBody3D.xml | 20 ++ scene/3d/physics/character_body_3d.cpp | 246 ++++++++++++++++++++++++- scene/3d/physics/character_body_3d.h | 22 +++ 3 files changed, 287 insertions(+), 1 deletion(-) diff --git a/doc/classes/CharacterBody3D.xml b/doc/classes/CharacterBody3D.xml index 35c87703c861..19cad9586db8 100644 --- a/doc/classes/CharacterBody3D.xml +++ b/doc/classes/CharacterBody3D.xml @@ -86,6 +86,13 @@ Returns the number of times the body collided and changed direction during the last call to [method move_and_slide]. + + + + Returns a smoothed position for visual elements like cameras. When [member step_smooth_enabled] is [code]true[/code], this position interpolates the vertical component during step-up and step-down events, preventing jarring camera movement when traversing stairs. The horizontal components match the actual physics position. + Use this for camera positioning instead of [member Node3D.global_position] when step handling is enabled. + + @@ -183,6 +190,19 @@ If [code]true[/code], during a jump against the ceiling, the body will slide, if [code]false[/code] it will be stopped and will fall vertically. + + If [code]true[/code], the body will automatically step up onto obstacles shorter than [member step_height] when calling [method move_and_slide]. Uses an up-forward-down trace algorithm similar to classic FPS engines. Only works when [member motion_mode] is [constant MOTION_MODE_GROUNDED]. + [b]Note:[/b] For best results, use a [CylinderShape3D] collider. [CapsuleShape3D] colliders have rounded bottoms that can cause step detection issues. + + + Maximum height of obstacles the body can step onto when [member step_enabled] is [code]true[/code]. Obstacles taller than this will block movement and cause sliding. + + + If [code]true[/code], the [method get_visual_position] method returns a smoothed position that interpolates during step events. This prevents jarring camera movement when traversing stairs. + + + Controls how quickly [method get_visual_position] catches up to the actual position during step events. Higher values result in faster, snappier camera response. This is a rate factor, not meters per second. + Vector pointing upwards, used to determine what is a wall and what is a floor (or a ceiling) when calling [method move_and_slide]. Defaults to [constant Vector3.UP]. As the vector will be normalized it can't be equal to [constant Vector3.ZERO], if you want all collisions to be reported as walls, consider using [constant MOTION_MODE_FLOATING] as [member motion_mode]. diff --git a/scene/3d/physics/character_body_3d.cpp b/scene/3d/physics/character_body_3d.cpp index 19bebc581ba4..789c9142367d 100644 --- a/scene/3d/physics/character_body_3d.cpp +++ b/scene/3d/physics/character_body_3d.cpp @@ -134,6 +134,9 @@ bool CharacterBody3D::move_and_slide() { } } + // Update visual position smoothing for step handling + _update_visual_position(delta); + return motion_results.size() > 0; } @@ -215,6 +218,15 @@ void CharacterBody3D::_move_and_slide_grounded(double p_delta, bool p_was_on_flo // Wall collision checks. if (result_state.wall && (motion_slide_up.dot(wall_normal) <= 0)) { + // Try step-up first if enabled and on floor (Unreal Engine 1 style) + if (step_enabled && p_was_on_floor && !vel_dir_facing_up) { + if (_try_step_up(result.remainder)) { + // Step succeeded - character has been moved to stepped position + // Break out of loop, snap_on_floor will handle final floor detection + break; + } + } + // Move on floor only checks. if (floor_block_on_wall) { // Needs horizontal motion from current motion instead of motion_slide_up @@ -399,6 +411,139 @@ void CharacterBody3D::_move_and_slide_grounded(double p_delta, bool p_was_on_flo } } +bool CharacterBody3D::_try_step_up(const Vector3 &p_remainder) { + // Unreal Engine 1 style step-up algorithm: + // 1. Move UP by step_height + // 2. Move FORWARD by remaining motion + // 3. Move DOWN by step_height + // 4. If we land on walkable floor, accept the step + + if (step_height <= 0) { + return false; + } + + const int STEP_CHECK_COUNT = 2; + const real_t WALL_MARGIN = 0.001; + + Transform3D start_transform = get_global_transform(); + Vector3 start_position = start_transform.origin; + + // Forward motion is the horizontal component of the remaining motion + // p_remainder is already in distance units (motion), not velocity + Vector3 forward_motion = p_remainder.slide(up_direction); + if (forward_motion.is_zero_approx()) { + return false; + } + + // Try stepping at decreasing heights (full, then half, etc.) + for (int i = 0; i < STEP_CHECK_COUNT; i++) { + Vector3 current_step_height = up_direction * (step_height - (step_height / STEP_CHECK_COUNT) * i); + + PhysicsServer3D::MotionParameters params; + PhysicsServer3D::MotionResult result; + + // Step 1: Test moving UP + Transform3D test_transform = start_transform; + params = PhysicsServer3D::MotionParameters(test_transform, current_step_height, margin); + params.recovery_as_collision = true; + + bool hit_ceiling = PhysicsServer3D::get_singleton()->body_test_motion(get_rid(), params, &result); + + // If we hit a ceiling (normal pointing down), try smaller step + if (hit_ceiling && result.collision_count > 0 && result.collisions[0].normal.dot(up_direction) < 0) { + continue; + } + + // Move test position up + test_transform.origin += current_step_height; + + // Step 2: Test moving FORWARD + params = PhysicsServer3D::MotionParameters(test_transform, forward_motion, margin); + params.recovery_as_collision = true; + + bool hit_wall_forward = PhysicsServer3D::get_singleton()->body_test_motion(get_rid(), params, &result); + + if (!hit_wall_forward) { + // No collision moving forward - test moving down + test_transform.origin += forward_motion; + + // Step 3: Test moving DOWN + params = PhysicsServer3D::MotionParameters(test_transform, -current_step_height, margin); + params.recovery_as_collision = true; + + bool hit_ground = PhysicsServer3D::get_singleton()->body_test_motion(get_rid(), params, &result); + + if (hit_ground && result.collision_count > 0) { + // Check if we hit a walkable floor + Vector3 ground_normal = result.collisions[0].normal; + if (ground_normal.angle_to(up_direction) <= floor_max_angle + FLOOR_ANGLE_THRESHOLD) { + // Success! Final position is where we hit the ground + Vector3 final_position = test_transform.origin + result.travel; + + // Apply the stepped position + Transform3D final_transform = start_transform; + final_transform.origin = final_position; + set_global_transform(final_transform); + + // Update floor state + collision_state.floor = true; + floor_normal = ground_normal; + last_motion = final_position - start_position; + + return true; + } + } + } else { + // Hit a wall while moving forward - try sliding along it + if (result.collision_count > 0) { + Vector3 wall_normal_fwd = result.collisions[0].normal; + + // Only handle vertical walls + if (Math::is_zero_approx(wall_normal_fwd.dot(up_direction))) { + // Slide along the wall + test_transform.origin += wall_normal_fwd * WALL_MARGIN; + Vector3 slide_motion = forward_motion.slide(wall_normal_fwd); + + params = PhysicsServer3D::MotionParameters(test_transform, slide_motion, margin); + params.recovery_as_collision = true; + + bool hit_after_slide = PhysicsServer3D::get_singleton()->body_test_motion(get_rid(), params, &result); + + if (!hit_after_slide) { + // Sliding worked - test moving down + test_transform.origin += slide_motion; + + params = PhysicsServer3D::MotionParameters(test_transform, -current_step_height, margin); + params.recovery_as_collision = true; + + bool hit_ground = PhysicsServer3D::get_singleton()->body_test_motion(get_rid(), params, &result); + + if (hit_ground && result.collision_count > 0) { + Vector3 ground_normal = result.collisions[0].normal; + if (ground_normal.angle_to(up_direction) <= floor_max_angle + FLOOR_ANGLE_THRESHOLD) { + // Success with wall slide + Vector3 final_position = test_transform.origin + result.travel; + + Transform3D final_transform = start_transform; + final_transform.origin = final_position; + set_global_transform(final_transform); + + collision_state.floor = true; + floor_normal = ground_normal; + last_motion = final_position - start_position; + + return true; + } + } + } + } + } + } + } + + return false; +} + void CharacterBody3D::_move_and_slide_floating(double p_delta) { Vector3 motion = velocity * p_delta; @@ -835,6 +980,90 @@ void CharacterBody3D::set_floor_snap_length(real_t p_floor_snap_length) { floor_snap_length = p_floor_snap_length; } +bool CharacterBody3D::is_step_enabled() const { + return step_enabled; +} + +void CharacterBody3D::set_step_enabled(bool p_enabled) { + step_enabled = p_enabled; +} + +real_t CharacterBody3D::get_step_height() const { + return step_height; +} + +void CharacterBody3D::set_step_height(real_t p_height) { + ERR_FAIL_COND(p_height < 0); + step_height = p_height; +} + +bool CharacterBody3D::is_step_smooth_enabled() const { + return step_smooth_enabled; +} + +void CharacterBody3D::set_step_smooth_enabled(bool p_enabled) { + step_smooth_enabled = p_enabled; +} + +real_t CharacterBody3D::get_step_smooth_speed() const { + return step_smooth_speed; +} + +void CharacterBody3D::set_step_smooth_speed(real_t p_speed) { + ERR_FAIL_COND(p_speed < 0); + step_smooth_speed = p_speed; +} + +void CharacterBody3D::_update_visual_position(double p_delta) { + real_t current_y = get_global_transform().origin.y; + + // Initialize on first call + if (!visual_position_initialized) { + visual_position_y = current_y; + visual_position_initialized = true; + return; + } + + // If smoothing disabled, just track actual position + if (!step_smooth_enabled || step_smooth_speed <= 0) { + visual_position_y = current_y; + return; + } + + // Only smooth while grounded - jumping/falling should have no camera lag + // This ensures only step-up and floor-snap (step-down) get smoothed + if (!collision_state.floor) { + visual_position_y = current_y; + return; + } + + // Calculate the difference between visual and actual Y + real_t diff = current_y - visual_position_y; + + // If difference is very small, snap to actual + if (Math::abs(diff) < 0.001) { + visual_position_y = current_y; + return; + } + + // Proportional smoothing: move a fraction of the remaining distance each frame + // This naturally handles both small and large offsets: + // - Small offset (single step): smooth, gradual movement + // - Large offset (multiple steps): faster catch-up, no sudden snaps + // step_smooth_speed acts as a rate factor (higher = faster catch-up) + real_t smooth_factor = step_smooth_speed * p_delta; + visual_position_y += diff * MIN(smooth_factor, 1.0); +} + +Vector3 CharacterBody3D::get_visual_position() const { + Vector3 pos = get_global_transform().origin; + if (step_smooth_enabled && visual_position_initialized) { + // Replace actual Y with smoothed Y + pos.y = visual_position_y; + } + return pos; +} + real_t CharacterBody3D::get_wall_min_slide_angle() const { return wall_min_slide_angle; } @@ -895,6 +1124,15 @@ void CharacterBody3D::_bind_methods() { ClassDB::bind_method(D_METHOD("set_floor_max_angle", "radians"), &CharacterBody3D::set_floor_max_angle); ClassDB::bind_method(D_METHOD("get_floor_snap_length"), &CharacterBody3D::get_floor_snap_length); ClassDB::bind_method(D_METHOD("set_floor_snap_length", "floor_snap_length"), &CharacterBody3D::set_floor_snap_length); + ClassDB::bind_method(D_METHOD("is_step_enabled"), &CharacterBody3D::is_step_enabled); + ClassDB::bind_method(D_METHOD("set_step_enabled", "enabled"), &CharacterBody3D::set_step_enabled); + ClassDB::bind_method(D_METHOD("get_step_height"), &CharacterBody3D::get_step_height); + ClassDB::bind_method(D_METHOD("set_step_height", "height"), &CharacterBody3D::set_step_height); + ClassDB::bind_method(D_METHOD("is_step_smooth_enabled"), &CharacterBody3D::is_step_smooth_enabled); + ClassDB::bind_method(D_METHOD("set_step_smooth_enabled", "enabled"), &CharacterBody3D::set_step_smooth_enabled); + ClassDB::bind_method(D_METHOD("get_step_smooth_speed"), &CharacterBody3D::get_step_smooth_speed); + ClassDB::bind_method(D_METHOD("set_step_smooth_speed", "speed"), &CharacterBody3D::set_step_smooth_speed); + ClassDB::bind_method(D_METHOD("get_visual_position"), &CharacterBody3D::get_visual_position); ClassDB::bind_method(D_METHOD("get_wall_min_slide_angle"), &CharacterBody3D::get_wall_min_slide_angle); ClassDB::bind_method(D_METHOD("set_wall_min_slide_angle", "radians"), &CharacterBody3D::set_wall_min_slide_angle); ClassDB::bind_method(D_METHOD("get_up_direction"), &CharacterBody3D::get_up_direction); @@ -936,6 +1174,12 @@ void CharacterBody3D::_bind_methods() { ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "floor_max_angle", PROPERTY_HINT_RANGE, "0,180,0.1,radians_as_degrees"), "set_floor_max_angle", "get_floor_max_angle"); ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "floor_snap_length", PROPERTY_HINT_RANGE, "0,1,0.01,or_greater,suffix:m"), "set_floor_snap_length", "get_floor_snap_length"); + ADD_GROUP("Step", "step_"); + ADD_PROPERTY(PropertyInfo(Variant::BOOL, "step_enabled"), "set_step_enabled", "is_step_enabled"); + ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "step_height", PROPERTY_HINT_RANGE, "0,1,0.01,or_greater,suffix:m"), "set_step_height", "get_step_height"); + ADD_PROPERTY(PropertyInfo(Variant::BOOL, "step_smooth_enabled"), "set_step_smooth_enabled", "is_step_smooth_enabled"); + ADD_PROPERTY(PropertyInfo(Variant::FLOAT, "step_smooth_speed", PROPERTY_HINT_RANGE, "1,30,0.5,or_greater"), "set_step_smooth_speed", "get_step_smooth_speed"); + ADD_GROUP("Moving Platform", "platform_"); ADD_PROPERTY(PropertyInfo(Variant::INT, "platform_on_leave", PROPERTY_HINT_ENUM, "Add Velocity,Add Upward Velocity,Do Nothing", PROPERTY_USAGE_DEFAULT), "set_platform_on_leave", "get_platform_on_leave"); ADD_PROPERTY(PropertyInfo(Variant::INT, "platform_floor_layers", PROPERTY_HINT_LAYERS_3D_PHYSICS), "set_platform_floor_layers", "get_platform_floor_layers"); @@ -957,7 +1201,7 @@ void CharacterBody3D::_validate_property(PropertyInfo &p_property) const { return; } if (motion_mode == MOTION_MODE_FLOATING) { - if (p_property.name.begins_with("floor_") || p_property.name == "up_direction" || p_property.name == "slide_on_ceiling") { + if (p_property.name.begins_with("floor_") || p_property.name.begins_with("step_") || p_property.name == "up_direction" || p_property.name == "slide_on_ceiling") { p_property.usage = PROPERTY_USAGE_NO_EDITOR; } } diff --git a/scene/3d/physics/character_body_3d.h b/scene/3d/physics/character_body_3d.h index 34c70d648e70..cc003f8b7c07 100644 --- a/scene/3d/physics/character_body_3d.h +++ b/scene/3d/physics/character_body_3d.h @@ -96,6 +96,20 @@ class CharacterBody3D : public PhysicsBody3D { real_t get_floor_snap_length(); void set_floor_snap_length(real_t p_floor_snap_length); + bool is_step_enabled() const; + void set_step_enabled(bool p_enabled); + + real_t get_step_height() const; + void set_step_height(real_t p_height); + + bool is_step_smooth_enabled() const; + void set_step_smooth_enabled(bool p_enabled); + + real_t get_step_smooth_speed() const; + void set_step_smooth_speed(real_t p_speed); + + Vector3 get_visual_position() const; + real_t get_wall_min_slide_angle() const; void set_wall_min_slide_angle(real_t p_radians); @@ -149,6 +163,12 @@ class CharacterBody3D : public PhysicsBody3D { real_t floor_snap_length = 0.1; real_t floor_max_angle = Math::deg_to_rad((real_t)45.0); real_t wall_min_slide_angle = Math::deg_to_rad((real_t)15.0); + bool step_enabled = false; + real_t step_height = 0.3; + bool step_smooth_enabled = true; + real_t step_smooth_speed = 10.0; + real_t visual_position_y = 0.0; + bool visual_position_initialized = false; Vector3 up_direction = Vector3(0.0, 1.0, 0.0); Vector3 velocity; Vector3 floor_normal; @@ -166,6 +186,8 @@ class CharacterBody3D : public PhysicsBody3D { void _move_and_slide_floating(double p_delta); void _move_and_slide_grounded(double p_delta, bool p_was_on_floor); + bool _try_step_up(const Vector3 &p_remainder); + void _update_visual_position(double p_delta); Ref _get_slide_collision(int p_bounce); Ref _get_last_slide_collision(); From 15119d35737ff049c4602d504bad8b8a5138aa8a Mon Sep 17 00:00:00 2001 From: Logan Spencer Date: Wed, 3 Jun 2026 06:37:55 -0500 Subject: [PATCH 2/5] GDScript: Add traits (interface contracts) with `implements` Introduce single-inheritance-friendly interface contracts to GDScript via the reserved `trait` keyword plus a new `implements` keyword: trait SimSystem: func get_value() -> int class Something extends RefCounted implements SimSystem: func get_value() -> int: return 42 A class may declare it satisfies one or more traits. The analyzer verifies every trait method is implemented with a compatible signature, and a trait is usable as a static type (a class is type-compatible with any trait it implements). Traits are compile-time-only contracts with no runtime identity -- they are lowered to untyped in the compiler -- so `implements` is analyzer-only metadata. Runtime `is`/`as` reflection against traits is intentionally deferred. - Tokenizer: new `implements` keyword (reuses the reserved `trait`). - Parser: file-level and inner trait declarations, `implements` lists, trait bodies restricted to bodyless (abstract) functions. - Analyzer: trait implementation + signature checks; trait/implementer type compatibility. - Compiler: lower trait-typed values to untyped at runtime. - Tests: analyzer error tests + a runtime feature test. Co-Authored-By: Claude Opus 4.8 (1M context) --- modules/gdscript/gdscript.cpp | 1 + modules/gdscript/gdscript_analyzer.cpp | 99 +++++++++++ modules/gdscript/gdscript_analyzer.h | 1 + modules/gdscript/gdscript_compiler.cpp | 8 + modules/gdscript/gdscript_parser.cpp | 159 +++++++++++++++++- modules/gdscript/gdscript_parser.h | 8 + modules/gdscript/gdscript_tokenizer.cpp | 3 + modules/gdscript/gdscript_tokenizer.h | 1 + .../analyzer/errors/trait_not_implemented.gd | 8 + .../analyzer/errors/trait_not_implemented.out | 2 + .../errors/trait_signature_mismatch.gd | 9 + .../errors/trait_signature_mismatch.out | 2 + .../analyzer/features/trait_implements.gd | 28 +++ .../analyzer/features/trait_implements.out | 4 + 14 files changed, 330 insertions(+), 3 deletions(-) create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/trait_not_implemented.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/trait_not_implemented.out create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/trait_signature_mismatch.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/errors/trait_signature_mismatch.out create mode 100644 modules/gdscript/tests/scripts/analyzer/features/trait_implements.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/features/trait_implements.out diff --git a/modules/gdscript/gdscript.cpp b/modules/gdscript/gdscript.cpp index 0fb2de89146e..bdf0967b4a7d 100644 --- a/modules/gdscript/gdscript.cpp +++ b/modules/gdscript/gdscript.cpp @@ -2614,6 +2614,7 @@ Vector GDScriptLanguage::get_reserved_words() const { "enum", "extends", "func", + "implements", "namespace", // Reserved for potential future use. "signal", "static", diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 5007a4338539..4eed9b5e0933 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -626,6 +626,11 @@ Error GDScriptAnalyzer::resolve_class_inheritance(GDScriptParser::ClassNode *p_c E->apply(parser, p_class, p_class->outer); } + // Resolve implemented trait types now so type-compatibility checks can see them later. + for (GDScriptParser::TypeNode *trait_type_node : p_class->implemented_traits) { + resolve_datatype(trait_type_node); + } + parser->current_class = previous_class; return OK; @@ -1567,9 +1572,93 @@ void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, co } } + // Verify that a concrete class fulfills every trait it claims to implement. + // Abstract classes are allowed to defer this to their concrete subclasses. + if (!p_class->is_abstract && p_class->implements_used) { + check_trait_implementations(p_class); + } + parser->current_class = previous_class; } +void GDScriptAnalyzer::check_trait_implementations(GDScriptParser::ClassNode *p_class) { + const String class_name = p_class->identifier == nullptr ? p_class->fqcn.get_file() : String(p_class->identifier->name); + + GDScriptParser::DataType self_type = p_class->get_datatype(); + self_type.is_meta_type = false; + + for (GDScriptParser::TypeNode *trait_type_node : p_class->implemented_traits) { + GDScriptParser::DataType trait_type = type_from_metatype(resolve_datatype(trait_type_node)); + + if (trait_type.kind != GDScriptParser::DataType::CLASS || trait_type.class_type == nullptr || !trait_type.class_type->is_trait) { + push_error(vformat(R"(Type "%s" used in "implements" is not a trait.)", trait_type.to_string()), trait_type_node); + continue; + } + + GDScriptParser::ClassNode *trait_class = trait_type.class_type; + + // Make sure the trait's members and their signatures are resolved. + resolve_class_interface(trait_class, p_class); + + const String trait_name = trait_class->identifier == nullptr ? trait_class->fqcn.get_file() : String(trait_class->identifier->name); + + for (const GDScriptParser::ClassNode::Member &member : trait_class->members) { + if (member.type != GDScriptParser::ClassNode::Member::FUNCTION) { + continue; + } + const StringName &function_name = member.function->identifier->name; + + // Expected signature, as declared by the trait. + GDScriptParser::DataType trait_return; + List trait_params; + int trait_default_count = 0; + BitField trait_flags = {}; + get_function_signature(p_class, false, trait_type, function_name, trait_return, trait_params, trait_default_count, trait_flags); + + // Actual signature, as found in the class or its base chain. + GDScriptParser::DataType impl_return; + List impl_params; + int impl_default_count = 0; + BitField impl_flags = {}; + if (!get_function_signature(p_class, false, self_type, function_name, impl_return, impl_params, impl_default_count, impl_flags) || impl_flags.has_flag(METHOD_FLAG_VIRTUAL_REQUIRED)) { + push_error(vformat(R"*(Class "%s" must implement "%s.%s()".)*", class_name, trait_name, function_name), p_class); + continue; + } + + bool valid = impl_flags.has_flag(METHOD_FLAG_STATIC) == trait_flags.has_flag(METHOD_FLAG_STATIC); + + // `[impl_min..impl_max]` must include `[trait_min..trait_max]`. + const int trait_min_argc = trait_params.size() - trait_default_count; + const int trait_max_argc = trait_flags.has_flag(METHOD_FLAG_VARARG) ? INT_MAX : trait_params.size(); + const int impl_min_argc = impl_params.size() - impl_default_count; + const int impl_max_argc = impl_flags.has_flag(METHOD_FLAG_VARARG) ? INT_MAX : impl_params.size(); + valid = valid && impl_min_argc <= trait_min_argc && trait_max_argc <= impl_max_argc; + + // Return type must be covariant. + if (valid && !trait_return.is_variant()) { + valid = is_type_compatible(trait_return, impl_return); + } + + // Parameter types must be contravariant. + const List::Element *trait_it = trait_params.front(); + const List::Element *impl_it = impl_params.front(); + while (valid && trait_it != nullptr && impl_it != nullptr) { + const GDScriptParser::DataType &trait_par = trait_it->get(); + const GDScriptParser::DataType &impl_par = impl_it->get(); + if (!trait_par.is_variant() && !impl_par.is_variant()) { + valid = is_type_compatible(impl_par, trait_par); + } + trait_it = trait_it->next(); + impl_it = impl_it->next(); + } + + if (!valid) { + push_error(vformat(R"*(The signature of "%s.%s()" does not match the signature declared by trait "%s".)*", class_name, function_name, trait_name), p_class); + } + } + } +} + void GDScriptAnalyzer::resolve_class_body(GDScriptParser::ClassNode *p_class, bool p_recursive) { resolve_class_body(p_class); @@ -6406,6 +6495,16 @@ bool GDScriptAnalyzer::check_type_compatibility(const GDScriptParser::DataType & if (src_class == p_target.class_type || src_class->fqcn == p_target.class_type->fqcn) { return true; } + // A class is compatible with a trait it (or a base class) implements. + if (p_target.class_type->is_trait) { + for (const GDScriptParser::TypeNode *trait_type_node : src_class->implemented_traits) { + const GDScriptParser::DataType impl_trait = trait_type_node->get_datatype(); + if (impl_trait.kind == GDScriptParser::DataType::CLASS && impl_trait.class_type != nullptr && + (impl_trait.class_type == p_target.class_type || impl_trait.class_type->fqcn == p_target.class_type->fqcn)) { + return true; + } + } + } src_class = src_class->base_type.class_type; } return false; diff --git a/modules/gdscript/gdscript_analyzer.h b/modules/gdscript/gdscript_analyzer.h index e2c3e9a32b18..7895a36cef94 100644 --- a/modules/gdscript/gdscript_analyzer.h +++ b/modules/gdscript/gdscript_analyzer.h @@ -77,6 +77,7 @@ class GDScriptAnalyzer { void resolve_class_interface(GDScriptParser::ClassNode *p_class, bool p_recursive); void resolve_class_body(GDScriptParser::ClassNode *p_class, const GDScriptParser::Node *p_source = nullptr); void resolve_class_body(GDScriptParser::ClassNode *p_class, bool p_recursive); + void check_trait_implementations(GDScriptParser::ClassNode *p_class); void resolve_function_signature(GDScriptParser::FunctionNode *p_function, const GDScriptParser::Node *p_source = nullptr, bool p_is_lambda = false); void resolve_function_body(GDScriptParser::FunctionNode *p_function, bool p_is_lambda = false); void resolve_node(GDScriptParser::Node *p_node, bool p_is_root = true); diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index f3e7cc8d02b1..af8e234885a6 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -92,6 +92,14 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D return GDScriptDataType(); } + // Traits are a compile-time-only contract with no runtime type identity. Their + // `implements` relationship is verified by the analyzer, but the VM knows nothing + // about it, so a trait-typed value must be left untyped at runtime to avoid + // spurious type checks (e.g. when passed to a trait-typed parameter). + if (!(p_handle_metatype && p_datatype.is_meta_type) && p_datatype.kind == GDScriptParser::DataType::CLASS && p_datatype.class_type != nullptr && p_datatype.class_type->is_trait) { + return GDScriptDataType(); + } + GDScriptDataType result; switch (p_datatype.kind) { diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 4777ff8c2820..157b257f62ce 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -804,6 +804,25 @@ void GDScriptParser::parse_program() { end_statement("superclass"); } break; + case GDScriptTokenizer::Token::TRAIT: + PUSH_PENDING_ANNOTATIONS_TO_HEAD; + advance(); + if (head->identifier != nullptr) { + push_error(R"("trait" can only be used once and not together with "class_name".)"); + } else { + parse_trait(); + } + break; + case GDScriptTokenizer::Token::IMPLEMENTS: + PUSH_PENDING_ANNOTATIONS_TO_HEAD; + advance(); + if (head->implements_used) { + push_error(R"("implements" can only be used once.)"); + } else { + parse_implements(); + end_statement("trait list"); + } + break; case GDScriptTokenizer::Token::TK_EOF: PUSH_PENDING_ANNOTATIONS_TO_HEAD; can_have_class_or_extends = false; @@ -847,6 +866,10 @@ void GDScriptParser::parse_program() { parse_class_body(true); + if (head->is_trait) { + apply_trait_member_rules(head); + } + head->end_line = current.end_line; head->end_column = current.end_column; @@ -949,7 +972,22 @@ bool GDScriptParser::has_class(const GDScriptParser::ClassNode *p_class) const { } GDScriptParser::ClassNode *GDScriptParser::parse_class(bool p_is_static) { + return parse_class_impl(p_is_static, false); +} + +GDScriptParser::ClassNode *GDScriptParser::parse_trait_class(bool p_is_static) { + return parse_class_impl(p_is_static, true); +} + +GDScriptParser::ClassNode *GDScriptParser::parse_class_impl(bool p_is_static, bool p_is_trait) { ClassNode *n_class = alloc_node(); + n_class->is_trait = p_is_trait; + if (p_is_trait) { + // A trait is an implicit abstract contract and cannot be instantiated. + n_class->is_abstract = true; + } + + const String keyword = p_is_trait ? "trait" : "class"; make_completion_context(COMPLETION_DECLARATION, n_class); @@ -957,7 +995,7 @@ GDScriptParser::ClassNode *GDScriptParser::parse_class(bool p_is_static) { current_class = n_class; n_class->outer = previous_class; - if (consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected identifier for the class name after "class".)")) { + if (consume(GDScriptTokenizer::Token::IDENTIFIER, vformat(R"(Expected identifier for the %s name after "%s".)", keyword, keyword))) { n_class->identifier = parse_identifier(); if (n_class->outer) { String fqcn = n_class->outer->fqcn; @@ -973,12 +1011,15 @@ GDScriptParser::ClassNode *GDScriptParser::parse_class(bool p_is_static) { if (match(GDScriptTokenizer::Token::EXTENDS)) { parse_extends(); } + if (match(GDScriptTokenizer::Token::IMPLEMENTS)) { + parse_implements(); + } - consume(GDScriptTokenizer::Token::COLON, R"(Expected ":" after class declaration.)"); + consume(GDScriptTokenizer::Token::COLON, vformat(R"(Expected ":" after %s declaration.)", keyword)); bool multiline = match(GDScriptTokenizer::Token::NEWLINE); - if (multiline && !consume(GDScriptTokenizer::Token::INDENT, R"(Expected indented block after class declaration.)")) { + if (multiline && !consume(GDScriptTokenizer::Token::INDENT, vformat(R"(Expected indented block after %s declaration.)", keyword))) { current_class = previous_class; complete_extents(n_class); return n_class; @@ -991,8 +1032,20 @@ GDScriptParser::ClassNode *GDScriptParser::parse_class(bool p_is_static) { parse_extends(); end_statement("superclass"); } + if (match(GDScriptTokenizer::Token::IMPLEMENTS)) { + if (n_class->implements_used) { + push_error(R"(Cannot use "implements" more than once in the same class.)"); + } + parse_implements(); + end_statement("trait list"); + } parse_class_body(multiline); + + if (p_is_trait) { + apply_trait_member_rules(n_class); + } + complete_extents(n_class); if (multiline) { @@ -1018,12 +1071,49 @@ void GDScriptParser::parse_class_name() { if (match(GDScriptTokenizer::Token::EXTENDS)) { // Allow extends on the same line. parse_extends(); + if (match(GDScriptTokenizer::Token::IMPLEMENTS)) { + parse_implements(); + } end_statement("superclass"); + } else if (match(GDScriptTokenizer::Token::IMPLEMENTS)) { + parse_implements(); + end_statement("trait list"); } else { end_statement("class_name statement"); } } +void GDScriptParser::parse_trait() { + current_class->is_trait = true; + // A trait is an implicit abstract contract and cannot be instantiated. + current_class->is_abstract = true; + + if (consume(GDScriptTokenizer::Token::IDENTIFIER, R"(Expected identifier for the trait name after "trait".)")) { + current_class->identifier = parse_identifier(); + current_class->fqcn = String(current_class->identifier->name); + } + + if (script_path.begins_with("res://") && script_path.contains("::")) { + push_error(R"("trait" isn't allowed in built-in scripts.)"); + } + + make_completion_context(COMPLETION_DECLARATION, current_class); + + if (match(GDScriptTokenizer::Token::EXTENDS)) { + // Allow extends on the same line. + parse_extends(); + if (match(GDScriptTokenizer::Token::IMPLEMENTS)) { + parse_implements(); + } + end_statement("superclass"); + } else if (match(GDScriptTokenizer::Token::IMPLEMENTS)) { + parse_implements(); + end_statement("trait list"); + } else { + end_statement("trait declaration"); + } +} + void GDScriptParser::parse_extends() { current_class->extends_used = true; @@ -1056,6 +1146,65 @@ void GDScriptParser::parse_extends() { } } +void GDScriptParser::parse_implements() { + current_class->implements_used = true; + + do { + TypeNode *trait_type = parse_type(false); + if (trait_type == nullptr) { + push_error(R"(Expected trait name after "implements".)"); + return; + } + current_class->implemented_traits.push_back(trait_type); + } while (match(GDScriptTokenizer::Token::COMMA)); +} + +void GDScriptParser::apply_trait_member_rules(ClassNode *p_trait) { + // A trait only declares a contract: bodyless (abstract) instance functions. + // Mark every function abstract (the "no body" check is reused from the analyzer) + // and reject any other kind of member for this first version. + for (const ClassNode::Member &member : p_trait->members) { + switch (member.type) { + case ClassNode::Member::FUNCTION: { + FunctionNode *function = member.function; + if (function->is_static) { + push_error(R"(Traits cannot declare static functions.)", function); + } + function->is_abstract = true; + } break; + case ClassNode::Member::UNDEFINED: + case ClassNode::Member::GROUP: + break; + default: { + const Node *member_node = p_trait; + switch (member.type) { + case ClassNode::Member::CLASS: + member_node = member.m_class; + break; + case ClassNode::Member::CONSTANT: + member_node = member.constant; + break; + case ClassNode::Member::SIGNAL: + member_node = member.signal; + break; + case ClassNode::Member::VARIABLE: + member_node = member.variable; + break; + case ClassNode::Member::ENUM: + member_node = member.m_enum; + break; + case ClassNode::Member::ENUM_VALUE: + member_node = member.enum_value.identifier; + break; + default: + break; + } + push_error(vformat(R"(Traits can only declare functions, but found %s "%s".)", member.get_type_name(), member.get_name()), member_node); + } break; + } + } +} + template void GDScriptParser::parse_class_member(T *(GDScriptParser::*p_parse_function)(bool), AnnotationInfo::TargetKind p_target, const String &p_member_kind, bool p_is_static) { advance(); @@ -1151,6 +1300,9 @@ void GDScriptParser::parse_class_body(bool p_is_multiline) { case GDScriptTokenizer::Token::CLASS: parse_class_member(&GDScriptParser::parse_class, AnnotationInfo::CLASS, "class"); break; + case GDScriptTokenizer::Token::TRAIT: + parse_class_member(&GDScriptParser::parse_trait_class, AnnotationInfo::CLASS, "trait"); + break; case GDScriptTokenizer::Token::ENUM: parse_class_member(&GDScriptParser::parse_enum, AnnotationInfo::NONE, "enum"); break; @@ -4331,6 +4483,7 @@ GDScriptParser::ParseRule *GDScriptParser::get_rule(GDScriptTokenizer::Token::Ty { nullptr, nullptr, PREC_NONE }, // ENUM, { nullptr, nullptr, PREC_NONE }, // EXTENDS, { &GDScriptParser::parse_lambda, nullptr, PREC_NONE }, // FUNC, + { nullptr, nullptr, PREC_NONE }, // IMPLEMENTS, { nullptr, &GDScriptParser::parse_binary_operator, PREC_CONTENT_TEST }, // TK_IN, { nullptr, &GDScriptParser::parse_type_test, PREC_TYPE_TEST }, // IS, { nullptr, nullptr, PREC_NONE }, // NAMESPACE, diff --git a/modules/gdscript/gdscript_parser.h b/modules/gdscript/gdscript_parser.h index 3d1b65492803..6aa1aa4397b1 100644 --- a/modules/gdscript/gdscript_parser.h +++ b/modules/gdscript/gdscript_parser.h @@ -753,12 +753,15 @@ class GDScriptParser { HashMap members_indices; ClassNode *outer = nullptr; bool extends_used = false; + bool implements_used = false; bool onready_used = false; bool is_abstract = false; + bool is_trait = false; bool has_static_data = false; bool annotated_static_unload = false; String extends_path; Vector extends; // List for indexing: extends A.B.C + Vector implemented_traits; // List for indexing: implements A, B.C DataType base_type; String fqcn; // Fully-qualified class name. Identifies uniquely any class in the project. #ifdef TOOLS_ENABLED @@ -1541,8 +1544,13 @@ class GDScriptParser { // Main blocks. void parse_program(); ClassNode *parse_class(bool p_is_static); + ClassNode *parse_trait_class(bool p_is_static); + ClassNode *parse_class_impl(bool p_is_static, bool p_is_trait); void parse_class_name(); + void parse_trait(); void parse_extends(); + void parse_implements(); + void apply_trait_member_rules(ClassNode *p_trait); void parse_class_body(bool p_is_multiline); template void parse_class_member(T *(GDScriptParser::*p_parse_function)(bool), AnnotationInfo::TargetKind p_target, const String &p_member_kind, bool p_is_static = false); diff --git a/modules/gdscript/gdscript_tokenizer.cpp b/modules/gdscript/gdscript_tokenizer.cpp index ad1e359f5fa3..44ba5de471fa 100644 --- a/modules/gdscript/gdscript_tokenizer.cpp +++ b/modules/gdscript/gdscript_tokenizer.cpp @@ -111,6 +111,7 @@ static const char *token_names[] = { "enum", // ENUM, "extends", // EXTENDS, "func", // FUNC, + "implements", // IMPLEMENTS, "in", // TK_IN, "is", // IS, "namespace", // NAMESPACE @@ -235,6 +236,7 @@ bool GDScriptTokenizer::Token::is_node_name() const { case FOR: case FUNC: case IF: + case IMPLEMENTS: case TK_IN: case IS: case MATCH: @@ -507,6 +509,7 @@ GDScriptTokenizer::Token GDScriptTokenizerText::annotation() { KEYWORD("func", Token::FUNC) \ KEYWORD_GROUP('i') \ KEYWORD("if", Token::IF) \ + KEYWORD("implements", Token::IMPLEMENTS) \ KEYWORD("in", Token::TK_IN) \ KEYWORD("is", Token::IS) \ KEYWORD_GROUP('m') \ diff --git a/modules/gdscript/gdscript_tokenizer.h b/modules/gdscript/gdscript_tokenizer.h index 4133db8ff067..79f1d45d23a4 100644 --- a/modules/gdscript/gdscript_tokenizer.h +++ b/modules/gdscript/gdscript_tokenizer.h @@ -116,6 +116,7 @@ class GDScriptTokenizer { ENUM, EXTENDS, FUNC, + IMPLEMENTS, TK_IN, // Conflict with WinAPI. IS, NAMESPACE, diff --git a/modules/gdscript/tests/scripts/analyzer/errors/trait_not_implemented.gd b/modules/gdscript/tests/scripts/analyzer/errors/trait_not_implemented.gd new file mode 100644 index 000000000000..372c27f5483e --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/trait_not_implemented.gd @@ -0,0 +1,8 @@ +class Something extends RefCounted implements SimSystem: + pass + +trait SimSystem: + func declared_func() -> bool + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/trait_not_implemented.out b/modules/gdscript/tests/scripts/analyzer/errors/trait_not_implemented.out new file mode 100644 index 000000000000..b2dbad333275 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/trait_not_implemented.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +>> ERROR at line 1: Class "Something" must implement "SimSystem.declared_func()". diff --git a/modules/gdscript/tests/scripts/analyzer/errors/trait_signature_mismatch.gd b/modules/gdscript/tests/scripts/analyzer/errors/trait_signature_mismatch.gd new file mode 100644 index 000000000000..91d182f005c2 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/trait_signature_mismatch.gd @@ -0,0 +1,9 @@ +class Something extends RefCounted implements SimSystem: + func declared_func() -> bool: + return true + +trait SimSystem: + func declared_func(value: int) -> bool + +func test(): + pass diff --git a/modules/gdscript/tests/scripts/analyzer/errors/trait_signature_mismatch.out b/modules/gdscript/tests/scripts/analyzer/errors/trait_signature_mismatch.out new file mode 100644 index 000000000000..ee68b60c4012 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/errors/trait_signature_mismatch.out @@ -0,0 +1,2 @@ +GDTEST_ANALYZER_ERROR +>> ERROR at line 1: The signature of "Something.declared_func()" does not match the signature declared by trait "SimSystem". diff --git a/modules/gdscript/tests/scripts/analyzer/features/trait_implements.gd b/modules/gdscript/tests/scripts/analyzer/features/trait_implements.gd new file mode 100644 index 000000000000..6bf9f452a58c --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/trait_implements.gd @@ -0,0 +1,28 @@ +# A trait declares a pure contract (bodyless functions). A class promises to +# satisfy it with `implements`, and can then be used statically as that type. +# (The trait is declared as an inner trait here so the whole example fits in one +# file; a top-level `trait Name` instead makes the file itself a named trait.) +class Something extends RefCounted implements SimSystem: + func get_value() -> int: + return 42 + +class OtherSystem extends RefCounted implements SimSystem: + func get_value() -> int: + return 7 + +trait SimSystem: + func get_value() -> int + +func use_system(system: SimSystem) -> int: + return system.get_value() + +func test(): + var something := Something.new() + + # An implementer is compatible with the trait type (Layer A static typing). + var system: SimSystem = something + print(system.get_value()) + + # Implementers can be passed where the trait type is expected. + print(use_system(something)) + print(use_system(OtherSystem.new())) diff --git a/modules/gdscript/tests/scripts/analyzer/features/trait_implements.out b/modules/gdscript/tests/scripts/analyzer/features/trait_implements.out new file mode 100644 index 000000000000..b17d60581b66 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/trait_implements.out @@ -0,0 +1,4 @@ +GDTEST_OK +42 +42 +7 From 72ac9971b98339d735363587c5de0cab8b3c5c51 Mon Sep 17 00:00:00 2001 From: Logan Spencer Date: Wed, 3 Jun 2026 06:43:41 -0500 Subject: [PATCH 3/5] GDScript: Test cross-file trait usage via global class name A file-level `trait` (no colon, members at file scope) registers as a global class and can be implemented and used as a static type from another file, including being passed where the trait type is expected. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tests/scripts/analyzer/features/trait_external.gd | 9 +++++++++ .../tests/scripts/analyzer/features/trait_external.out | 2 ++ .../analyzer/features/trait_external_system.notest.gd | 3 +++ 3 files changed, 14 insertions(+) create mode 100644 modules/gdscript/tests/scripts/analyzer/features/trait_external.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/features/trait_external.out create mode 100644 modules/gdscript/tests/scripts/analyzer/features/trait_external_system.notest.gd diff --git a/modules/gdscript/tests/scripts/analyzer/features/trait_external.gd b/modules/gdscript/tests/scripts/analyzer/features/trait_external.gd new file mode 100644 index 000000000000..5894a37d1854 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/trait_external.gd @@ -0,0 +1,9 @@ +class Impl extends RefCounted implements ExternalSystem: + func get_value() -> int: + return 99 + +func use_it(s: ExternalSystem) -> int: + return s.get_value() + +func test(): + print(use_it(Impl.new())) diff --git a/modules/gdscript/tests/scripts/analyzer/features/trait_external.out b/modules/gdscript/tests/scripts/analyzer/features/trait_external.out new file mode 100644 index 000000000000..29f1b8af6ec0 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/trait_external.out @@ -0,0 +1,2 @@ +GDTEST_OK +99 diff --git a/modules/gdscript/tests/scripts/analyzer/features/trait_external_system.notest.gd b/modules/gdscript/tests/scripts/analyzer/features/trait_external_system.notest.gd new file mode 100644 index 000000000000..83eda55a0e61 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/trait_external_system.notest.gd @@ -0,0 +1,3 @@ +trait ExternalSystem + +func get_value() -> int From f69c696aca5a662d419c25fb045e1618e21c4adc Mon Sep 17 00:00:00 2001 From: Logan Spencer Date: Wed, 3 Jun 2026 07:16:15 -0500 Subject: [PATCH 4/5] GDScript: Clearer error for a file-level trait written with ":" A file-level trait follows `class_name` syntax (no colon, members at file scope). Writing `trait Foo:` at file scope now reports an actionable message instead of the generic "Expected end of statement after trait declaration". Co-Authored-By: Claude Opus 4.8 (1M context) --- modules/gdscript/gdscript_parser.cpp | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/modules/gdscript/gdscript_parser.cpp b/modules/gdscript/gdscript_parser.cpp index 157b257f62ce..a05eedcd80a3 100644 --- a/modules/gdscript/gdscript_parser.cpp +++ b/modules/gdscript/gdscript_parser.cpp @@ -1102,16 +1102,20 @@ void GDScriptParser::parse_trait() { if (match(GDScriptTokenizer::Token::EXTENDS)) { // Allow extends on the same line. parse_extends(); - if (match(GDScriptTokenizer::Token::IMPLEMENTS)) { - parse_implements(); - } - end_statement("superclass"); - } else if (match(GDScriptTokenizer::Token::IMPLEMENTS)) { + } + if (match(GDScriptTokenizer::Token::IMPLEMENTS)) { parse_implements(); - end_statement("trait list"); - } else { - end_statement("trait declaration"); } + + if (check(GDScriptTokenizer::Token::COLON)) { + // Common mistake: writing a file-level trait like a nested one (`trait Foo:`). + // A file-level trait names the whole file (like `class_name`), so its members + // live at the top level of the file rather than in an indented block. + push_error(R"(A file-level trait does not use ":". Declare its members at the top level of the file (like "class_name"), or use a nested "trait Name:" inside a class.)"); + return; + } + + end_statement("trait declaration"); } void GDScriptParser::parse_extends() { From 5d43de8fc2be75721fbd552c2b443e46ef6d7ef1 Mon Sep 17 00:00:00 2001 From: Logan Spencer Date: Wed, 3 Jun 2026 07:16:15 -0500 Subject: [PATCH 5/5] GDScript: Support `is` and `as` for traits at runtime `obj is SomeTrait` and `obj as SomeTrait` now recognise trait implementation: `is` is true (and `as` returns the object) when the object's class or any base class declares the trait via `implements`; otherwise `is` is false and `as` returns null. The compiled GDScript records the traits each class implements, and the `is`/`as` opcodes consult that list while walking the base-script chain. Trait types remain lowered to untyped for variables/parameters, except in the `is`/`as` paths, which need the real trait script type. - GDScript: store `implemented_traits` per class. - Compiler: populate it; emit the real trait type for `is`/`as` (not lowered). - VM: check the trait list in OPCODE_TYPE_TEST_SCRIPT and OPCODE_CAST_TO_SCRIPT. - Analyzer: allow `is`/`as` against a trait regardless of the operand's static type. - Test: runtime is/as coverage including inheritance and a non-implementer. Co-Authored-By: Claude Opus 4.8 (1M context) --- modules/gdscript/gdscript.h | 1 + modules/gdscript/gdscript_analyzer.cpp | 13 ++++++++- modules/gdscript/gdscript_compiler.cpp | 27 +++++++++++++++---- modules/gdscript/gdscript_compiler.h | 2 +- modules/gdscript/gdscript_vm.cpp | 26 ++++++++++++++++++ .../scripts/analyzer/features/trait_is_as.gd | 24 +++++++++++++++++ .../scripts/analyzer/features/trait_is_as.out | 6 +++++ 7 files changed, 92 insertions(+), 7 deletions(-) create mode 100644 modules/gdscript/tests/scripts/analyzer/features/trait_is_as.gd create mode 100644 modules/gdscript/tests/scripts/analyzer/features/trait_is_as.out diff --git a/modules/gdscript/gdscript.h b/modules/gdscript/gdscript.h index c36c418fc497..0421921294a8 100644 --- a/modules/gdscript/gdscript.h +++ b/modules/gdscript/gdscript.h @@ -104,6 +104,7 @@ class GDScript : public Script { HashMap constants; HashMap member_functions; HashMap> subclasses; + Vector> implemented_traits; // Traits declared via `implements`, used by `is`/`as` at runtime. HashMap _signals; Dictionary rpc_config; diff --git a/modules/gdscript/gdscript_analyzer.cpp b/modules/gdscript/gdscript_analyzer.cpp index 4eed9b5e0933..01b0fbee8b70 100644 --- a/modules/gdscript/gdscript_analyzer.cpp +++ b/modules/gdscript/gdscript_analyzer.cpp @@ -3903,6 +3903,13 @@ void GDScriptAnalyzer::reduce_cast(GDScriptParser::CastNode *p_cast) { valid = is_type_compatible(cast_type, op_type) || is_type_compatible(op_type, cast_type); } + // A trait can be implemented by otherwise-unrelated types, so `as SomeTrait` is a + // legitimate runtime-checked cast even when the static types are unrelated. + if (!valid && cast_type.kind == GDScriptParser::DataType::CLASS && cast_type.class_type != nullptr && cast_type.class_type->is_trait) { + valid = true; + mark_node_unsafe(p_cast); + } + if (!valid) { push_error(vformat(R"(Invalid cast. Cannot convert from "%s" to "%s".)", op_type.to_string(), cast_type.to_string()), p_cast->cast_type); } @@ -5299,7 +5306,11 @@ void GDScriptAnalyzer::reduce_type_test(GDScriptParser::TypeTestNode *p_type_tes return; } - if (!is_type_compatible(test_type, operand_type) && !is_type_compatible(operand_type, test_type)) { + // A trait can be implemented by otherwise-unrelated types, so `is SomeTrait` is always a + // legitimate runtime check and must not be rejected based on the operand's static type. + const bool test_is_trait = test_type.kind == GDScriptParser::DataType::CLASS && test_type.class_type != nullptr && test_type.class_type->is_trait; + + if (!test_is_trait && !is_type_compatible(test_type, operand_type) && !is_type_compatible(operand_type, test_type)) { if (operand_type.is_hard_type()) { push_error(vformat(R"(Expression is of type "%s" so it can't be of type "%s".)", operand_type.to_string(), test_type.to_string()), p_type_test->operand); } else { diff --git a/modules/gdscript/gdscript_compiler.cpp b/modules/gdscript/gdscript_compiler.cpp index af8e234885a6..e3095269d5d6 100644 --- a/modules/gdscript/gdscript_compiler.cpp +++ b/modules/gdscript/gdscript_compiler.cpp @@ -87,7 +87,7 @@ void GDScriptCompiler::_set_error(const String &p_error, const GDScriptParser::N } } -GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner, bool p_handle_metatype) { +GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner, bool p_handle_metatype, bool p_lower_traits) { if (!p_datatype.is_set() || !p_datatype.is_hard_type() || p_datatype.is_coroutine) { return GDScriptDataType(); } @@ -95,8 +95,10 @@ GDScriptDataType GDScriptCompiler::_gdtype_from_datatype(const GDScriptParser::D // Traits are a compile-time-only contract with no runtime type identity. Their // `implements` relationship is verified by the analyzer, but the VM knows nothing // about it, so a trait-typed value must be left untyped at runtime to avoid - // spurious type checks (e.g. when passed to a trait-typed parameter). - if (!(p_handle_metatype && p_datatype.is_meta_type) && p_datatype.kind == GDScriptParser::DataType::CLASS && p_datatype.class_type != nullptr && p_datatype.class_type->is_trait) { + // spurious type checks (e.g. when passed to a trait-typed parameter). The + // exceptions are `is`/`as` and the per-class trait list, which need the real + // trait script type to perform the runtime check; those pass `p_lower_traits = false`. + if (p_lower_traits && !(p_handle_metatype && p_datatype.is_meta_type) && p_datatype.kind == GDScriptParser::DataType::CLASS && p_datatype.class_type != nullptr && p_datatype.class_type->is_trait) { return GDScriptDataType(); } @@ -588,7 +590,8 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code } break; case GDScriptParser::Node::CAST: { const GDScriptParser::CastNode *cn = static_cast(p_expression); - GDScriptDataType cast_type = _gdtype_from_datatype(cn->get_datatype(), codegen.script, false); + // `p_lower_traits = false`: a cast to a trait needs the real trait script type so the VM can check `implements`. + GDScriptDataType cast_type = _gdtype_from_datatype(cn->get_datatype(), codegen.script, false, false); GDScriptCodeGenerator::Address result; if (cast_type.has_type()) { @@ -970,7 +973,8 @@ GDScriptCodeGenerator::Address GDScriptCompiler::_parse_expression(CodeGen &code GDScriptCodeGenerator::Address result = codegen.add_temporary(_gdtype_from_datatype(type_test->get_datatype(), codegen.script)); GDScriptCodeGenerator::Address operand = _parse_expression(codegen, r_error, type_test->operand); - GDScriptDataType test_type = _gdtype_from_datatype(type_test->test_datatype, codegen.script, false); + // `p_lower_traits = false`: `is SomeTrait` needs the real trait script type so the VM can check `implements`. + GDScriptDataType test_type = _gdtype_from_datatype(type_test->test_datatype, codegen.script, false, false); if (r_error) { return GDScriptCodeGenerator::Address(); } @@ -3022,6 +3026,19 @@ Error GDScriptCompiler::_prepare_compilation(GDScript *p_script, const GDScriptP } Error GDScriptCompiler::_compile_class(GDScript *p_script, const GDScriptParser::ClassNode *p_class, bool p_keep_state) { + // Record the traits this class implements so `is`/`as` can verify them at runtime. + p_script->implemented_traits.clear(); + for (GDScriptParser::TypeNode *trait_node : p_class->implemented_traits) { + // `p_lower_traits = false` keeps the real trait script type instead of lowering it to untyped. + GDScriptDataType trait_type = _gdtype_from_datatype(trait_node->get_datatype(), p_script, false, false); + if (trait_type.kind == GDScriptDataType::GDSCRIPT || trait_type.kind == GDScriptDataType::SCRIPT) { + Ref trait_script = Object::cast_to(trait_type.script_type); + if (trait_script.is_valid()) { + p_script->implemented_traits.push_back(trait_script); + } + } + } + // Compile member functions, getters, and setters. for (int i = 0; i < p_class->members.size(); i++) { const GDScriptParser::ClassNode::Member &member = p_class->members[i]; diff --git a/modules/gdscript/gdscript_compiler.h b/modules/gdscript/gdscript_compiler.h index ce832f249d0a..69b7c8c3e94d 100644 --- a/modules/gdscript/gdscript_compiler.h +++ b/modules/gdscript/gdscript_compiler.h @@ -147,7 +147,7 @@ class GDScriptCompiler { void _set_error(const String &p_error, const GDScriptParser::Node *p_node); - GDScriptDataType _gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner, bool p_handle_metatype = true); + GDScriptDataType _gdtype_from_datatype(const GDScriptParser::DataType &p_datatype, GDScript *p_owner, bool p_handle_metatype = true, bool p_lower_traits = true); GDScriptCodeGenerator::Address _parse_expression(CodeGen &codegen, Error &r_error, const GDScriptParser::ExpressionNode *p_expression, bool p_root = false, bool p_initializer = false); GDScriptCodeGenerator::Address _parse_match_pattern(CodeGen &codegen, Error &r_error, const GDScriptParser::PatternNode *p_pattern, const GDScriptCodeGenerator::Address &p_value_addr, const GDScriptCodeGenerator::Address &p_type_addr, const GDScriptCodeGenerator::Address &p_previous_test, bool p_is_first, bool p_is_nested); diff --git a/modules/gdscript/gdscript_vm.cpp b/modules/gdscript/gdscript_vm.cpp index b470b15872f2..9b9aee953f05 100644 --- a/modules/gdscript/gdscript_vm.cpp +++ b/modules/gdscript/gdscript_vm.cpp @@ -976,6 +976,19 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a result = true; break; } + // A class is also `is` any trait it (or a base class) implements. + GDScript *gdscript_ptr = Object::cast_to(script_ptr); + if (gdscript_ptr) { + for (const Ref &trait : gdscript_ptr->implemented_traits) { + if (trait.ptr() == script_type) { + result = true; + break; + } + } + if (result) { + break; + } + } script_ptr = script_ptr->get_base_script().ptr(); } } @@ -1718,6 +1731,19 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a valid = true; break; } + // A class can also be cast to any trait it (or a base class) implements. + GDScript *gdscript_ptr = Object::cast_to(src_type); + if (gdscript_ptr) { + for (const Ref &trait : gdscript_ptr->implemented_traits) { + if (trait.ptr() == base_type) { + valid = true; + break; + } + } + if (valid) { + break; + } + } src_type = src_type->get_base_script().ptr(); } } diff --git a/modules/gdscript/tests/scripts/analyzer/features/trait_is_as.gd b/modules/gdscript/tests/scripts/analyzer/features/trait_is_as.gd new file mode 100644 index 000000000000..3245fd5ed1f0 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/trait_is_as.gd @@ -0,0 +1,24 @@ +class Something extends RefCounted implements SimSystem: + func get_value() -> int: + return 42 + +class NotASystem extends RefCounted: + func get_value() -> int: + return 0 + +class DerivedSomething extends Something: + pass + +trait SimSystem: + func get_value() -> int + +func test(): + var s := Something.new() + var n := NotASystem.new() + var d := DerivedSomething.new() + + print(s is SimSystem) # true + print(n is SimSystem) # false + print(d is SimSystem) # true (inherited from Something) + print((s as SimSystem) != null) # true + print((n as SimSystem) == null) # true diff --git a/modules/gdscript/tests/scripts/analyzer/features/trait_is_as.out b/modules/gdscript/tests/scripts/analyzer/features/trait_is_as.out new file mode 100644 index 000000000000..79dd629ee676 --- /dev/null +++ b/modules/gdscript/tests/scripts/analyzer/features/trait_is_as.out @@ -0,0 +1,6 @@ +GDTEST_OK +true +false +true +true +true