Various updates #156
Various updates #156
Conversation
|
WalkthroughThis pull request introduces significant enhancements across various modules of the project. It includes improvements in character movement, interaction, weapon mechanics, and visual effects. The changes also optimize performance, improve code clarity, and introduce new features like mesh slicing, charging weapons, and customizable bullet holes. Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (24)
- addons/nodot/Nodot.gd (1)
- addons/nodot/detection/ViewCone3D.gd (2)
- addons/nodot/hud/CrossHair.gd (2)
- addons/nodot/interaction/Interaction3D.gd (3)
- addons/nodot/kits/FirstPerson/FirstPersonCharacter.gd (1)
- addons/nodot/kits/FirstPerson/FirstPersonItem.gd (6)
- addons/nodot/kits/FirstPerson/FirstPersonItemsContainer.gd (3)
- addons/nodot/kits/FirstPerson/FirstPersonMouseInput.gd (5)
- addons/nodot/kits/Locomotion3D/CharacterClimb3D.gd (1)
- addons/nodot/kits/Locomotion3D/CharacterMover3D.gd (1)
- addons/nodot/kits/Locomotion3D/CharacterSwim3D.gd (3)
- addons/nodot/kits/Locomotion3D/CharacterZeroGravity3D.gd (1)
- addons/nodot/kits/NodotCharacter3D.gd (2)
- addons/nodot/mesh/MeshShatter.gd (1)
- addons/nodot/mesh/MeshSlicer.gd (1)
- addons/nodot/physics/Mover3D.gd (3)
- addons/nodot/scenes/menus/main_menu.tscn (1)
- addons/nodot/shaders/lava.gdshader (1)
- addons/nodot/textures/spotlight_2.png.import (1)
- addons/nodot/utility/UniqueSet.gd (1)
- addons/nodot/utility/WindowSizeConnector.gd (1)
- addons/nodot/weapons/BulletHole.gd (2)
- addons/nodot/weapons/HitScan3D.gd (2)
- addons/nodot/weapons/Magazine.gd (5)
Files ignored due to filter (1)
- addons/nodot/textures/spotlight_2.png
Files not summarized due to errors (1)
- addons/nodot/mesh/MeshSlicer.gd (diff tokens exceeds limit)
Files not reviewed due to errors (10)
- addons/nodot/hud/CrossHair.gd (no response)
- addons/nodot/kits/FirstPerson/FirstPersonItem.gd (no response)
- addons/nodot/kits/FirstPerson/FirstPersonCharacter.gd (no response)
- addons/nodot/kits/FirstPerson/FirstPersonItemsContainer.gd (no response)
- addons/nodot/kits/FirstPerson/FirstPersonMouseInput.gd (no response)
- addons/nodot/kits/NodotCharacter3D.gd (no response)
- addons/nodot/kits/Locomotion3D/CharacterZeroGravity3D.gd (no response)
- addons/nodot/mesh/MeshShatter.gd (no response)
- addons/nodot/utility/WindowSizeConnector.gd (no response)
- addons/nodot/weapons/BulletHole.gd (no response)
Files skipped from review due to trivial changes (4)
- addons/nodot/scenes/menus/main_menu.tscn
- addons/nodot/textures/spotlight_2.png.import
- addons/nodot/utility/UniqueSet.gd
- addons/nodot/mesh/MeshSlicer.gd (diff too large)
Review comments generated (23)
- Review: 13
- LGTM: 10
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| var new_y_velocity = clamp(character.velocity.y - submerged_gravity * delta, -3.0, 3.0) | ||
| character.velocity.y = lerp(character.velocity.y, new_y_velocity, 0.025) |
There was a problem hiding this comment.
The new logic for calculating new_y_velocity and updating character.velocity.y improves the control over the character's y-axis velocity. It now clamps the velocity between -3.0 and 3.0 before applying a lerp function, which should make the swimming movement smoother and more predictable.
- character.velocity.y -= submerged_gravity * delta
+ var new_y_velocity = clamp(character.velocity.y - submerged_gravity * delta, -3.0, 3.0)
+ character.velocity.y = lerp(character.velocity.y, new_y_velocity, 0.025)| shader_type spatial; | ||
|
|
||
| uniform float timeScaleFactor = 0.04; | ||
| uniform vec3 color: source_color = vec3(1.0, 0.0, 0.0); | ||
| uniform sampler2D underlay_texture: repeat_enable, filter_linear; | ||
| uniform sampler2D emission_texture: repeat_enable, filter_linear; | ||
| uniform sampler2D normal_texture: repeat_enable, filter_linear; | ||
| uniform sampler2D noise_texture: repeat_enable, filter_linear; | ||
|
|
||
| float localTime() { | ||
| return TIME * timeScaleFactor; | ||
| } | ||
|
|
||
| mat2 makem2(float theta) { | ||
| float c = cos(theta); | ||
| float s = sin(theta); | ||
| return mat2(vec2(c, -s), vec2(s, c)); | ||
| } | ||
|
|
||
| float noise(vec2 x) { | ||
| return texture(noise_texture, x * 0.01).r; // Assuming the noise is in the red channel | ||
| } | ||
|
|
||
| vec2 gradn(vec2 p) { | ||
| float ep = 0.09; | ||
| float gradx = noise(vec2(p.x + ep, p.y)) - noise(vec2(p.x - ep, p.y)); | ||
| float grady = noise(vec2(p.x, p.y + ep)) - noise(vec2(p.x, p.y - ep)); | ||
| return vec2(gradx, grady); | ||
| } | ||
|
|
||
| float flow(vec2 p) { | ||
| float z = 2.0; | ||
| float rz = 0.0; | ||
| vec2 bp = p; | ||
|
|
||
| for (float i = 1.0; i < 7.0; ++i) { | ||
| vec2 offset = vec2(sin(localTime() * 0.6), cos(localTime() * 1.9)) * 0.1; // Oscillating offset | ||
| vec2 gr = gradn(i * (p + offset) * 0.34); | ||
| gr *= makem2(sin(localTime() * 6.0) * PI); | ||
| p += gr * 0.5; | ||
| rz += (sin(noise(p) * 7.0) * 0.5 + 0.5) / z; | ||
| z *= 1.4; | ||
| p *= 2.0; | ||
| bp *= 1.9; | ||
| } | ||
| return rz; | ||
| } | ||
|
|
||
| void fragment() { | ||
|
|
||
| float rz = flow(UV); | ||
| vec3 shader_col = color / rz; // Red color from the shader effect | ||
| shader_col = pow(shader_col, vec3(1.4)); | ||
|
|
||
| vec3 texture_col = texture(underlay_texture, UV).rgb; // Color from the additional texture | ||
| vec3 final_col = mix(shader_col, texture_col, 0.0); | ||
| vec2 uv = vec2(UV.x, UV.y + localTime() / 10.0); | ||
|
|
||
| ALBEDO = texture(emission_texture, uv).rgb; | ||
| NORMAL_MAP = texture(normal_texture, uv).rgb; | ||
| EMISSION = final_col; | ||
| } |
There was a problem hiding this comment.
The shader code seems to be well written and follows good practices. However, there is a lack of comments explaining the logic behind the calculations and functions. This could make it difficult for other developers to understand the purpose and workings of this shader. It would be beneficial to add comments explaining the purpose of each function and the logic behind the calculations.
Also, there's no newline at the end of the file. While this isn't a critical issue, it's generally considered good practice to end files with a newline. This can prevent potential issues with certain tools that expect or require a newline at the end of a file.
+ // Add newline at the end of the file
}
+ | var charge_amount: float = 0.0; | ||
| var is_charging: bool = false; | ||
|
|
||
|
|
||
| func _physics_process(delta: float) -> void: | ||
| if is_charging: | ||
| charge_amount += charge_speed * delta; | ||
|
|
There was a problem hiding this comment.
The charging logic seems fine, but it's not clear what happens when the charge amount exceeds a certain limit. If there is a maximum charge amount, you should add a check to ensure charge_amount does not exceed this limit.
if is_charging:
- charge_amount += charge_speed * delta;
+ charge_amount = min(charge_amount + charge_speed * delta, max_charge_amount);| if charge_weapon: | ||
| if time_since_last_fired < fire_rate: return; | ||
| if not is_charging: | ||
| GlobalSignal.trigger_signal("charge_started", charge_speed); | ||
| is_charging = true; | ||
| time_since_last_fired = 0 | ||
| return; |
There was a problem hiding this comment.
The signal charge_started is being emitted globally using GlobalSignal.trigger_signal. This could potentially lead to issues if other parts of the code are also listening for a charge_started signal. It would be better to emit this signal from the instance itself to avoid potential conflicts.
- GlobalSignal.trigger_signal("charge_started", charge_speed);
+ emit_signal("charge_started", charge_speed);
krazyjakee
force-pushed
the
various-updates
branch
from
a35b3a8
to
7fc9cec
Compare
There was a problem hiding this comment.
Files selected (30)
- addons/nodot/Nodot.gd (1)
- addons/nodot/detection/ViewCone3D.gd (2)
- addons/nodot/hud/CrossHair.gd (2)
- addons/nodot/interaction/Interaction3D.gd (3)
- addons/nodot/kits/FirstPerson/FirstPersonCharacter.gd (1)
- addons/nodot/kits/FirstPerson/FirstPersonItem.gd (6)
- addons/nodot/kits/FirstPerson/FirstPersonItemsContainer.gd (3)
- addons/nodot/kits/FirstPerson/FirstPersonMouseInput.gd (5)
- addons/nodot/kits/Locomotion3D/CharacterClimb3D.gd (1)
- addons/nodot/kits/Locomotion3D/CharacterJump3D.gd (1)
- addons/nodot/kits/Locomotion3D/CharacterMover3D.gd (1)
- addons/nodot/kits/Locomotion3D/CharacterSwim3D.gd (3)
- addons/nodot/kits/Locomotion3D/CharacterZeroGravity3D.gd (1)
- addons/nodot/kits/NodotCharacter3D.gd (2)
- addons/nodot/mesh/GridMeshInstance3D.gd (1)
- addons/nodot/mesh/MeshShatter.gd (1)
- addons/nodot/mesh/MeshSlicer.gd (1)
- addons/nodot/physics/Mover3D.gd (3)
- addons/nodot/physics/WaterArea3D.gd (1)
- addons/nodot/scenes/menus/main_menu.tscn (1)
- addons/nodot/shaders/lava.gdshader (1)
- addons/nodot/sound/SFXArea3D.gd (1)
- addons/nodot/sound/SFXPlayer.gd (1)
- addons/nodot/sound/SFXPlayer3D.gd (2)
- addons/nodot/textures/spotlight_2.png.import (1)
- addons/nodot/utility/UniqueSet.gd (1)
- addons/nodot/utility/WindowSizeConnector.gd (1)
- addons/nodot/weapons/BulletHole.gd (2)
- addons/nodot/weapons/HitScan3D.gd (2)
- addons/nodot/weapons/Magazine.gd (5)
Files ignored due to filter (1)
- addons/nodot/textures/spotlight_2.png
Files not summarized due to errors (1)
- addons/nodot/mesh/MeshSlicer.gd (diff tokens exceeds limit)
Files not reviewed due to errors (14)
- addons/nodot/hud/CrossHair.gd (no response)
- addons/nodot/kits/FirstPerson/FirstPersonCharacter.gd (no response)
- addons/nodot/kits/FirstPerson/FirstPersonItem.gd (no response)
- addons/nodot/kits/FirstPerson/FirstPersonItemsContainer.gd (no response)
- addons/nodot/kits/FirstPerson/FirstPersonMouseInput.gd (no response)
- addons/nodot/kits/Locomotion3D/CharacterZeroGravity3D.gd (no response)
- addons/nodot/kits/NodotCharacter3D.gd (no response)
- addons/nodot/mesh/GridMeshInstance3D.gd (no response)
- addons/nodot/mesh/MeshShatter.gd (no response)
- addons/nodot/physics/Mover3D.gd (no response)
- addons/nodot/physics/WaterArea3D.gd (no response)
- addons/nodot/shaders/lava.gdshader (no response)
- addons/nodot/weapons/BulletHole.gd (no response)
- addons/nodot/weapons/Magazine.gd (no response)
Files skipped from review due to trivial changes (3)
- addons/nodot/sound/SFXPlayer.gd
- addons/nodot/utility/UniqueSet.gd
- addons/nodot/mesh/MeshSlicer.gd (diff too large)
Review comments generated (22)
- Review: 15
- LGTM: 7
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| @@ -5,6 +5,7 @@ class_name Nodot extends Node | |||
|
|
|||
| ## Returns all children of a given type | |||
| static func get_children_of_type(parent: Node, type: Variant) -> Array[Node]: | |||
| if !parent: return [] | |||
There was a problem hiding this comment.
Good addition of a null check for the parent node. This will prevent potential null reference exceptions when calling get_children() on a null object.
+ if !parent: return []This change improves the robustness of the code by handling edge cases where the parent node might be null.
| var collider = get_collider() | ||
| if !is_instance_valid(collider): | ||
| if is_instance_valid(carried_body): | ||
| carry_end(); | ||
| return | ||
|
|
||
| emit_signal("interacted", collider, get_collision_point(), get_collision_normal()) | ||
| if collider.has_method("interact"): | ||
| collider.interact() | ||
| GlobalSignal.trigger_signal("interacted", [collider, get_collision_point(), get_collision_normal()]); | ||
| else: | ||
| var collider = get_collider() | ||
| if !is_instance_valid(collider): | ||
| return | ||
|
|
||
| emit_signal("interacted", collider, get_collision_point(), get_collision_normal()) | ||
| if collider.has_method("interact"): | ||
| collider.interact() | ||
| if is_instance_valid(carried_body): | ||
| carry_end(); | ||
| else: | ||
| carry_begin(collider) | ||
|
|
||
|
|
||
| func _physics_process(delta): | ||
| if is_instance_valid(carried_body): | ||
| if not multiplayer.is_server(): return | ||
|
|
||
| var carry_position = global_transform.origin - global_transform.basis.z.normalized() * (carry_distance + carried_body_width) | ||
| var current_carry_distance = carried_body.global_position.distance_to(global_position) | ||
| if current_carry_distance > carry_distance + max_carry_distance: | ||
| carry_end(); | ||
| return | ||
| if Input.is_mouse_button_pressed(MOUSE_BUTTON_LEFT): | ||
| throw(); | ||
| return; | ||
| var speed = carried_body.global_position.distance_to(carry_position) * 500 | ||
| carried_body.linear_velocity = carried_body.global_transform.origin.direction_to(carry_position) * speed * delta | ||
| if not is_close_body_carry: | ||
| var carry_position = global_transform.origin - global_transform.basis.z.normalized() * (carry_distance + carried_body_width) | ||
| var current_carry_distance = carried_body.global_position.distance_to(global_position) | ||
| if current_carry_distance > carry_distance + max_carry_distance: | ||
| carry_end(); | ||
| return | ||
| if Input.is_mouse_button_pressed(MOUSE_BUTTON_LEFT): | ||
| throw(); | ||
| return; | ||
| var speed = carried_body.global_position.distance_to(carry_position) * 600 | ||
| carried_body.linear_velocity = carried_body.global_transform.origin.direction_to(carry_position) * speed * delta | ||
| else: | ||
| var carry_position = carry_position_node.global_position; | ||
| carried_body.set_collision_layer_value(1, false); | ||
| carried_body.global_position = carry_position; | ||
| if Input.is_mouse_button_pressed(MOUSE_BUTTON_LEFT): | ||
| var prev_carried_body = carried_body; | ||
| throw(); | ||
| await get_tree().create_timer(0.35).timeout; | ||
| prev_carried_body.set_collision_layer_value(1, carried_body_prev_mask); | ||
| return; | ||
| var rotate_speed: float = 10.0 * delta | ||
| carried_body.global_rotation.x = lerp_angle(carried_body.global_rotation.x, 0.0, rotate_speed) | ||
| carried_body.global_rotation.z = lerp_angle(carried_body.global_rotation.z, 0.0, rotate_speed) | ||
| carried_body.global_rotation.y = lerp_angle(carried_body.global_rotation.y, global_rotation.y, rotate_speed) | ||
|
|
||
| var collider = get_collider() | ||
| if is_instance_valid(collider): | ||
| if is_instance_valid(last_focussed_collider) and collider != last_focussed_collider: | ||
| if last_focussed_collider.has_method("unfocussed"): | ||
| last_focussed_collider.unfocussed(); | ||
| if collider.has_method("focussed"): | ||
| last_focussed_collider = collider; | ||
| collider.focussed(); | ||
| else: | ||
| if last_focussed_collider and last_focussed_collider.has_method("unfocussed"): | ||
| last_focussed_collider.unfocussed(); |
There was a problem hiding this comment.
The _input and _physics_process functions have been significantly refactored. The changes seem to improve the handling of carried bodies and interactions with colliders, which is a positive change. However, there's a potential issue in line 102 where a timer is created but not stored or managed. This could lead to orphaned timers if the function is called multiple times before the timer expires. Consider storing the timer in a variable and ensuring it's properly cleaned up.
- await get_tree().create_timer(0.35).timeout;
+ var timer = get_tree().create_timer(0.35)
+ await timer.timeout;
+ timer.queue_free();|
|
||
| func carry_begin(collider: Node): | ||
| if enable_pickup and is_instance_valid(collider) and collider is RigidBody3D and collider.mass <= max_mass: | ||
| carried_body = collider | ||
| is_close_body_carry = carried_body.has_meta("carry_close") and carried_body.get_meta("carry_close") | ||
| carried_body_gravity_scale = collider.gravity_scale | ||
| var carried_body_mesh: MeshInstance3D = Nodot.get_first_child_of_type(carried_body, MeshInstance3D) | ||
| if carried_body_mesh: | ||
| var mesh_size = carried_body_mesh.get_aabb().size | ||
| carried_body_width = max(mesh_size.x * carried_body_mesh.scale.x, mesh_size.y * carried_body_mesh.scale.y, mesh_size.z * carried_body_mesh.scale.z) | ||
| carried_body.gravity_scale = 0.0 | ||
| carried_body_prev_mask = carried_body.get_collision_mask_value(1); | ||
| emit_signal("carry_started", carried_body) | ||
|
|
||
| GlobalSignal.trigger_signal("carry_started", carried_body); | ||
|
|
||
|
|
||
| func carry_end(): | ||
| if is_instance_valid(carried_body): | ||
| if is_close_body_carry: | ||
| carried_body.set_collision_layer_value(1, carried_body_prev_mask); | ||
| carried_body.gravity_scale = carried_body_gravity_scale | ||
| emit_signal("carry_ended", carried_body) | ||
| GlobalSignal.trigger_signal("carry_ended", carried_body); | ||
| carried_body = null | ||
|
|
||
|
|
||
| func throw(): | ||
| if is_instance_valid(carried_body): | ||
| carried_body.gravity_scale = carried_body_gravity_scale | ||
|
|
||
| carried_body.apply_force(-global_transform.basis.z * throw_force); | ||
| emit_signal("carry_ended", carried_body) | ||
| emit_signal("carry_ended", carried_body); | ||
| GlobalSignal.trigger_signal("carry_ended", carried_body); | ||
| carried_body = null | ||
|
|
||
|
|
There was a problem hiding this comment.
In the carry_begin, carry_end, and throw functions, new variables and checks have been added to handle the carrying of bodies more robustly. These changes appear to be beneficial for the overall functionality of the system. However, there's a repetition of code in lines 161-162 and 170-171. You might consider creating a separate function to emit these signals to adhere to the DRY principle.
- emit_signal("carry_ended", carried_body)
- GlobalSignal.trigger_signal("carry_ended", carried_body);
+ emit_carry_ended_signals(carried_body)
# New function
func emit_carry_ended_signals(body: RigidBody3D):
emit_signal("carry_ended", body)
GlobalSignal.trigger_signal("carry_ended", body);|
|
||
|
|
||
| var removed_bodies: Array[Node] = [] | ||
| for detected_body in detected_bodies: | ||
| if !detected_bodies_this_pass.has(detected_body): | ||
| removed_bodies.append(detected_body) | ||
|
|
||
| for removed_body in removed_bodies: | ||
| if removed_body.has_method("undetected"): | ||
| removed_body.undetected(self) | ||
| emit_signal("body_lost", removed_body) | ||
| var i = detected_bodies.find(removed_body) | ||
| detected_bodies.remove_at(i) |
There was a problem hiding this comment.
The new code has improved the logic for managing detected bodies by removing the unnecessary size check for detected_bodies_this_pass. This change enhances performance as it avoids an extra operation when the array is empty. Good job!
- if detected_bodies_this_pass.size() > 0:
+
var removed_bodies: Array[Node] = []
for detected_body in detected_bodies:
if !detected_bodies_this_pass.has(detected_body):
removed_bodies.append(detected_body)
for removed_body in removed_bodies:
if removed_body.has_method("undetected"):
removed_body.undetected(self)
emit_signal("body_lost", removed_body)
var i = detected_bodies.find(removed_body)
detected_bodies.remove_at(i)| @@ -67,6 +67,6 @@ func physics(delta: float): | |||
| if is_on_floor and was_on_floor == false: | |||
| sm.set_state(state_ids["idle"]) | |||
|
|
|||
| was_on_floor = is_on_floor | |||
| was_on_floor = is_on_floor != null | |||
There was a problem hiding this comment.
The change from was_on_floor = is_on_floor to was_on_floor = is_on_floor != null might introduce a logical error. If is_on_floor is a boolean, comparing it with null will always result in true. This means was_on_floor will always be true, regardless of the actual value of is_on_floor.
- was_on_floor = is_on_floor != null
+ was_on_floor = is_on_floor| @@ -21,7 +21,7 @@ class_name SFXPlayer3D extends AudioStreamPlayer3D | |||
| ## The name of the signal | |||
| var trigger_signal: String = "" | |||
| var rng: RandomNumberGenerator = RandomNumberGenerator.new() | |||
| var original_volume = volume_db | |||
| var original_volume: float | |||
There was a problem hiding this comment.
The change from var original_volume = volume_db to var original_volume: float might cause a problem if the volume_db value was intended to be the initial value for original_volume. If that's the case, you should initialize original_volume with volume_db again.
- var original_volume: float
+ var original_volume = volume_db| @@ -18,5 +18,5 @@ func _ready() -> void: | |||
| func _on_window_resized(new_size: Vector2) -> void: | |||
| for node in target_nodes: | |||
| if "size" in node: | |||
| node.size = new_size * size_multiplier | |||
| node.set_deferred("size", new_size * size_multiplier) | |||
There was a problem hiding this comment.
The change from direct assignment to set_deferred is a good one. It ensures that the size property will be updated after all current frame's calculations are done, which can prevent potential issues with immediate changes in the middle of a frame's calculations.
- node.size = new_size * size_multiplier
+ node.set_deferred("size", new_size * size_multiplier)However, it's important to note that using set_deferred might cause a one-frame delay in updating the size. If this delay could cause any visual or functional issues in your game, you might need to find another solution.
| var player | ||
|
|
||
| func _get_configuration_warnings() -> PackedStringArray: | ||
| var warnings: PackedStringArray = [] | ||
| if !Nodot.get_first_child_of_type(self, SFXPlayer3D): | ||
| warnings.append("Should contain at least one SFXPlayer3D") | ||
| if !Nodot.get_first_child_of_type(self, SFXPlayer3D) and !Nodot.get_first_child_of_type(self, SFXPlayer): | ||
| warnings.append("Should contain at least one SFXPlayer3D or SFXPlayer") | ||
| return warnings | ||
|
|
||
| func _enter_tree(): | ||
| player = Nodot.get_first_child_of_type(self, SFXPlayer3D) | ||
| if !player: | ||
| player = Nodot.get_first_child_of_type(self, SFXPlayer) |
There was a problem hiding this comment.
The variable player is not typed. It's a good practice to use static typing in GDScript as it can help prevent potential bugs and improve code readability.
- var player
+ var player: Node| func _enter_tree(): | ||
| player = Nodot.get_first_child_of_type(self, SFXPlayer3D) | ||
| if !player: | ||
| player = Nodot.get_first_child_of_type(self, SFXPlayer) |
There was a problem hiding this comment.
The function _enter_tree() is missing error handling for the case when neither SFXPlayer3D nor SFXPlayer are found. This could lead to unexpected behavior if player remains null. Consider adding an error message or throwing an exception in this case.
func _enter_tree():
player = Nodot.get_first_child_of_type(self, SFXPlayer3D)
if !player:
player = Nodot.get_first_child_of_type(self, SFXPlayer)
+ if !player:
+ push_error("No SFXPlayer3D or SFXPlayer found in the tree.")
if is_editor or !enabled or !player: return
connect("body_entered", _detect_character_and_play)| signal target_hit(hit_target: HitTarget); | ||
|
|
||
| var rng: RandomNumberGenerator = RandomNumberGenerator.new() | ||
|
|
||
| var charge_amount: float = 0.0; | ||
|
|
||
| ## Execute the hitscan | ||
| ## TODO: Typehint this when nullable static types are supported. https://github.com/godotengine/godot-proposals/issues/162 | ||
| @rpc("any_peer", "call_local") | ||
| func action(dispatch_count: int = 1) -> void: | ||
| func action(dispatch_count: int = 1, _charge_amount: float = 0.0) -> void: | ||
| if enabled: | ||
| charge_amount = _charge_amount; |
There was a problem hiding this comment.
The action function now accepts an additional parameter _charge_amount which is used to set the charge_amount variable. However, there's no check for negative values. If a negative value is passed, it could lead to unexpected behavior. Consider adding a check to ensure that _charge_amount is not less than 0.
func action(dispatch_count: int = 1, _charge_amount: float = 0.0) -> void:
if enabled:
+ if _charge_amount < 0.0:
+ print("Error: Charge amount cannot be negative.")
+ return
charge_amount = _charge_amount;
Includes various updates across different files in the project. The changes focus on improving the code structure and functionality of specific components.
In
addons/nodot/Nodot.gd, a check for the validity of the parent node has been added to prevent potential issues.In
addons/nodot/detection/ViewCone3D.gd, the logic for managing detected bodies has been optimized to enhance performance.In
addons/nodot/hud/CrossHair.gd, additional functionality has been introduced to handle the crosshair sprite and its scaling more efficiently.In
addons/nodot/interaction/Interaction3D.gd, improvements have been made to the interaction and carrying mechanisms, including the handling of carried bodies and interactions with colliders.These updates aim to enhance the project's functionality, maintainability, and performance. Each change contributes to a more robust and efficient codebase, ultimately improving the overall quality of the project.
Summary by CodeRabbit
set_deferredin WindowSizeConnector.