feat: scripting input api #335
Conversation
…type + feat: now handle the memento save/restore pattern
…ctor into multiple source file
…does not process inputs anymore
…unction of the engine for different scene props
…nt size via a beginRender function that also handles the docking update and visibility update
…d on the size of the image drawn
Introduces structures for describing components and their fields, including field types and metadata. This allows for dynamic UI generation and component manipulation based on data provided from C#. Removes the NativeComponents enum in C# as it is replaced by the more generic ComponentDescription.
Changes component type retrieval to use ComponentType directly. This removes the reliance on type indices during retrieval, simplifying the process and potentially improving performance. Also provides function to retrieve type indices if needed.
Adds the ability to display and edit properties of C# components within the inspector window. This allows for dynamic inspection of components defined outside of the core engine. It achieves this by creating a `TypeErasedProperty` class that handles the display of type-erased fields, using component descriptions from the coordinator. Also fixes an issue with undo/redo actions.
Introduces a mechanism to reflect component fields for UI generation. This includes defining Field, FieldArray, and FieldType to describe component fields. The changes enable dynamic UI creation based on component structure. Removes unused NativeComponents.cs
Adds the DOTNET_INSTALL_DIR environment variable to the build and CodeQL workflows. This ensures that the .NET SDK is installed in a consistent and predictable location, improving the reliability of the build process.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes introduce conditional compilation and build configuration options to enable or disable C# scripting support throughout the codebase. New input handling features are added to the managed C# API, including keyboard and mouse input queries and a demo system. The CMake build scripts and native/managed code are updated to support optional scripting and to expose new input-related native API functions to managed code. Changes
Sequence Diagram(s)sequenceDiagram
participant ManagedCode as C# Script (InputDemoSystem)
participant Input as Input (C#)
participant NativeInterop as NativeInterop (C#)
participant NativeAPI as NativeApiCallbacks (C++)
participant Engine as Engine Input System (C++)
ManagedCode->>Input: IsKeyPressed(KeyCode.Space)
Input->>NativeInterop: IsKeyPressed(KeyCode.Space)
NativeInterop->>NativeAPI: NxIsKeyPressed(keycode)
NativeAPI->>Engine: event::isKeyPressed(keycode)
Engine-->>NativeAPI: bool
NativeAPI-->>NativeInterop: bool
NativeInterop-->>Input: bool
Input-->>ManagedCode: bool
ManagedCode->>Input: GetMousePosition()
Input->>NativeInterop: GetMousePosition()
NativeInterop->>NativeAPI: NxGetMousePosition(Vector2*)
NativeAPI->>Engine: event::getMousePosition()
Engine-->>NativeAPI: Vector2
NativeAPI-->>NativeInterop: Vector2
NativeInterop-->>Input: Vector2
Input-->>ManagedCode: Vector2
sequenceDiagram
participant BuildSystem as CMake
participant ScriptingCode as Scripting Sources/Headers
participant App as Application/Editor
BuildSystem->>BuildSystem: Option NEXO_BUILD_SCRIPTING=ON/OFF
alt Scripting enabled
BuildSystem->>ScriptingCode: Include scripting sources, define NEXO_SCRIPTING_ENABLED
App->>ScriptingCode: Use scripting system, include scripting headers
else Scripting disabled
BuildSystem->>ScriptingCode: Exclude scripting sources, do not define NEXO_SCRIPTING_ENABLED
App->>ScriptingCode: Do not include scripting headers or logic
end
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (2)
editor/src/Editor.cpp (1)
558-558: Fix unused variable warning.The pipeline indicates that variable
appon line 558 is unused. Consider removing the declaration or using the variable if it's needed.- Application& app = Application::getInstance();engine/src/Application.cpp (1)
113-157: Fix duplicate key callback registration.The key callback is registered twice with identical logic (lines 113-134 and 136-157). This appears to be unrelated to the current PR but should be addressed to prevent potential issues.
- m_window->setKeyCallback([this](const int key, const int action, const int mods) { - event::EventKey eventKey; - eventKey.keycode = key; - eventKey.mods = mods; - switch (action) - { - case GLFW_PRESS: { - eventKey.action = event::KeyAction::PRESSED; - break; - } - case GLFW_RELEASE: { - eventKey.action = event::KeyAction::RELEASED; - break; - } - case GLFW_REPEAT: { - eventKey.action = event::KeyAction::REPEAT; - break; - } - default: return; - } - m_eventManager->emitEvent<event::EventKey>(std::make_shared<event::EventKey>(eventKey)); - });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (15)
CMakeLists.txt(2 hunks)editor/main.cpp(1 hunks)editor/src/Editor.cpp(1 hunks)engine/CMakeLists.txt(2 hunks)engine/src/Application.cpp(3 hunks)engine/src/Application.hpp(2 hunks)engine/src/scripting/managed/CMakeLists.txt(3 hunks)engine/src/scripting/managed/Input.cs(1 hunks)engine/src/scripting/managed/KeyCode.cs(1 hunks)engine/src/scripting/managed/NativeInterop.cs(3 hunks)engine/src/scripting/managed/Scripts/InputDemoSystem.cs(1 hunks)engine/src/scripting/native/ManagedTypedef.hpp(1 hunks)engine/src/scripting/native/NativeApi.cpp(2 hunks)engine/src/scripting/native/NativeApi.hpp(2 hunks)engine/src/scripting/native/Scripting.cpp(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Build, test and Package
editor/src/Editor.cpp
[warning] 558-558: Warning: unused variable 'app'.
engine/src/scripting/native/NativeApi.hpp
[warning] 55-108: Warning: 'cdecl' attribute ignored on multiple function declarations.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (c-cpp/gcc) on ubuntu-latest
🔇 Additional comments (25)
engine/src/scripting/native/ManagedTypedef.hpp (1)
48-48: LGTM! Clean addition following existing patterns.The Vector2 typedef is consistent with the existing Vector3 and Vector4 definitions and supports the new input API for mouse position interop.
editor/main.cpp (1)
28-31: LGTM! Proper conditional compilation for modular scripting support.The conditional inclusion of scripting headers correctly supports the optional scripting feature introduced by the
NEXO_BUILD_SCRIPTINGCMake option.CMakeLists.txt (2)
15-15: LGTM! Well-named CMake option with sensible default.The
NEXO_BUILD_SCRIPTINGoption clearly communicates its purpose and the default value of ON maintains existing behavior.
101-104: LGTM! Correct conditional inclusion of scripting components.The conditional logic properly includes managed scripting subdirectory and dependencies only when scripting is enabled, supporting modular builds.
editor/src/Editor.cpp (1)
232-235: LGTM! Appropriate conditional compilation for scripting-specific functionality.The conditional guards around
registerTypeErasedProperties()correctly exclude scripting-related initialization when scripting is disabled.engine/src/scripting/managed/CMakeLists.txt (2)
30-31: LGTM! New input-related C# files properly added.The addition of
Input.cs,KeyCode.cs, andScripts/InputDemoSystem.csaligns well with the scripting input API objectives and follows the existing project structure.Also applies to: 45-45
75-75: Improved build configuration handling.The change from
$<CONFIG>to the explicit generator expression ensures that only valid Debug/Release configurations are passed to dotnet build, preventing potential build issues with unsupported configuration names.engine/CMakeLists.txt (2)
67-75: Excellent modularization of scripting sources.The conditional inclusion of scripting-related source files based on
NEXO_BUILD_SCRIPTINGprovides clean separation and allows builds without scripting dependencies.
131-136: Proper conditional linking and compilation definitions.The conditional linking of nethost and definition of
NEXO_SCRIPTING_ENABLEDmacro are correctly gated by the same condition, ensuring consistency across the build system.engine/src/scripting/managed/KeyCode.cs (1)
77-82: MouseButton enum looks correct.The MouseButton enum uses standard 0-2 values which align with typical mouse button conventions.
engine/src/Application.hpp (1)
41-45: Consistent conditional compilation for scripting system.The conditional compilation guards around both the forward declaration and member variable ensure that scripting components are cleanly excluded when
NEXO_SCRIPTING_ENABLEDis not defined.Also applies to: 266-268
engine/src/Application.cpp (1)
34-37: Well-implemented conditional scripting integration.The conditional compilation properly guards all scripting-related functionality with appropriate fallbacks when scripting is disabled, maintaining clean separation of concerns.
Also applies to: 202-204, 209-223, 295-297
engine/src/scripting/managed/Input.cs (1)
23-33: Well-designed input abstraction.The static class design with Unity-style API provides a familiar and clean interface for managed scripts. The delegation to
NativeInteropmaintains proper separation of concerns.engine/src/scripting/native/NativeApi.cpp (3)
167-174: Good defensive programming with null pointer check.The null pointer validation in
NxGetMousePositionprevents potential crashes and follows safe coding practices.
177-202: Well-structured callback initialization.The initialization pattern using
initializeNativeApiCallbacks()and static initialization helper ensures callbacks are properly set up. This approach guarantees initialization occurs before any managed code attempts to use the callbacks.
147-175: Clean implementation of input API functions.The input functions properly delegate to the event system and maintain consistent patterns with existing API functions. The implementation is straightforward and follows established conventions.
engine/src/scripting/managed/NativeInterop.cs (3)
86-99: Consistent delegate declarations following established patterns.The new input delegates maintain consistency with existing function pointer declarations and use appropriate calling conventions.
366-379: Proper unsafe pointer handling for mouse position.The unsafe pointer operation is necessary for the native interop and is correctly implemented with proper null initialization and error handling.
314-364: Consistent exception handling across input methods.All input wrapper methods follow the established pattern of try-catch blocks with appropriate fallback values, maintaining consistency with existing interop methods.
engine/src/scripting/managed/Scripts/InputDemoSystem.cs (3)
73-86: Excellent state management for key press detection.The transition detection from
!_spaceWasPressedtospacePressedcorrectly implements edge-triggered input handling, preventing spam actions from held keys. The cooldown mechanism adds additional protection.
145-179: Well-implemented cube spawning with nice features.The random positioning, color generation, and rainbow effect using HSV conversion demonstrate good use of the input API. The logging provides helpful feedback to users.
181-214: Clean HSV to RGB color conversion.The mathematical implementation correctly handles the HSV color space conversion with proper conditional logic for each hue sector.
engine/src/scripting/native/NativeApi.hpp (3)
57-67: Improved ApiCallback initialization support.The addition of a default constructor and assignment operator allows for more flexible initialization of callback function pointers, which is essential for the new initialization pattern.
111-129: Well-structured callback interface extension.The NativeApiCallbacks struct properly extends the existing pattern with new input-related callbacks, maintaining type safety with the ApiCallback template.
104-108: Address the pipeline warning about 'cdecl' attribute.The pipeline indicates that the 'cdecl' attribute is being ignored on these function declarations. This could be due to conflicting calling convention specifications or platform-specific issues.
#!/bin/bash # Check for conflicting calling convention specifications echo "Searching for calling convention conflicts in native API files..." rg -A 3 -B 3 "__cdecl|__stdcall|NEXO_CALL" engine/src/scripting/native/ echo "" echo "Checking for platform-specific calling convention definitions..." rg -A 5 -B 5 "NEXO_CALL" engine/src/scripting/native/NativeApi.hpp
Screencast.from.25-06-2025.20.10.03.webmPressing space to spawn block, delete to delete, arrow to move @Thyodas |
Thyodas
force-pushed
the
feat/scripting-ui
branch
from
426f095 to
a80586c
Compare
Thyodas
force-pushed
the
feat/scripting-ui
branch
2 times, most recently
from
bf93660 to
04b0cc2
Compare
Summary by CodeRabbit
New Features
Improvements
Bug Fixes