Feat/resource manager

[Saverio976]

• Please check if the PR fulfills these requirements

• The commit message follows our guidelines
• Tests for the changes have been added (for bug fixes / features)
• Docs have been added / updated (for bug fixes / features)
• Norm of the code has been respected

• In what state is your pull request?

• Ready to merge / Waiting a reviwer to see my work.
• Work In Progress (WIP) / My work is not finish but i want daily reviews. (Draft)
• CI Review / I want to see what the CI thinks of my work.
• Need feedback / Just to have feedback on what i produced. (May not be merged)

• What kind of change does this PR introduce? (You can choose multiple)

• Bug fix
• Feature request
• New / Updated documentation
• Testing CI ( Make the pull request in draft mode)

• What is the current behavior? (link an issue based on the kind of change this pr introduce)

• What is the new behavior (if this is a feature change)?

https://x-r-g-b.atlassian.net/browse/RB-128?atlOrigin=eyJpIjoiMTlhNjFlYTMzM2Q3NGY0YjlmZjBiNjgxMzZiZjQwODIiLCJwIjoiaiJ9

• Other information:

Summary by CodeRabbit

• New Feature: Introduced a ResourcesManager class to manage and convert file paths, improving the handling of resources across the application.
• Improvement: Enhanced the build process with options to enable packaging and control the use of Clang-Tidy, providing more flexibility during development.
• Improvement: Updated the Audio, Graphics, and Json classes to use the new ResourcesManager, ensuring consistent path handling.
• Documentation: Updated the installation guide for Windows, making it more accurate and user-friendly.
• Chore: Updated the GitHub workflow and scripts to better manage dependencies and build configurations.
• New Feature: Added the Boost filesystem library to the project, enhancing file and path operations.
• Chore: Updated .gitignore to ignore the "r-type-linux.tar.gz" file, keeping the repository clean.

[coderabbitai]

Walkthrough

This pull request introduces a new ResourcesManager class to manage resource paths, enabling the conversion of relative paths to absolute paths. It also adds flexibility to the build process by allowing the use of Clang-Tidy to be toggled on or off. Additionally, it includes changes to the GitHub workflow, CMakeLists, and installation scripts to accommodate these new features.

Changes

File(s)	Summary
.github/workflows/release.yml	Added --recurse-submodules flag to git clone command to ensure all necessary submodules are cloned.
CMakeLists.txt, deps/boost/CMakeLists.txt	Added installation step for assets, modified Clang-Tidy logic, added PACKAGED compile definition, and included Boost filesystem library.
docs/user-guide/installation/source/windows.md, scripts/*.sh, scripts/*.ps1	Updated commands for changing directories, installing dependencies, and building the project. Added options for interactive installation and disabling Clang-Tidy.
src/Client/Raylib/Audio/*.cpp, src/Client/Raylib/Graphics/Graphics.cpp	Updated to use ResourcesManager for file path conversion. Added getRealPath() functions and _realPath variables.
src/ECS/*.cpp, src/ECS/*.hpp	Introduced ResourcesManager class for resource path management. Updated Json class to use ResourcesManager and made several functions static.
src/main_client.cpp, src/main_server.cpp	Added initialization of ResourcesManager.
.gitignore	Added "r-type-linux.tar.gz" to the repository.

🐇 "In the land of code, where the shadows lie,

A rabbit hopped, with a twinkle in its eye.

Paths were tangled, build was slow,

But with these changes, we're good to go!

Now the paths are clear, and the build is quick,

Thanks to the rabbit, with its magic trick!" 🎩✨

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• Mention @coderabbitai in any review comment for bot assistance.
• Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

• Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
• Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
  resume reviews, enter @coderabbitai resume as any issue or review comment.

[coderabbitai]

Review Status

Actionable comments generated: 5

Commits

Files that changed from the base of the PR and between 82021b8 and 22638df.

Files selected for processing (19)

• .github/workflows/release.yml (1 hunks)
• CMakeLists.txt (3 hunks)
• deps/boost/CMakeLists.txt (1 hunks)
• docs/user-guide/installation/source/windows.md (1 hunks)
• scripts/bundle-linux.sh (1 hunks)
• scripts/bundle-macos.sh (1 hunks)
• scripts/bundle-windows.ps1 (1 hunks)
• scripts/compil.sh (1 hunks)
• scripts/install-deps-windows.ps1 (1 hunks)
• src/Client/Raylib/Audio/Audio.cpp (4 hunks)
• src/Client/Raylib/Audio/Audio.hpp (2 hunks)
• src/Client/Raylib/Graphics/Graphics.cpp (3 hunks)
• src/ECS/CMakeLists.txt (2 hunks)
• src/ECS/Json.cpp (7 hunks)
• src/ECS/Json.hpp (3 hunks)
• src/ECS/ResourcesManager.cpp (1 hunks)
• src/ECS/ResourcesManager.hpp (1 hunks)
• src/main_client.cpp (2 hunks)
• src/main_server.cpp (2 hunks)

Files skipped from review due to trivial changes (5)

• deps/boost/CMakeLists.txt
• docs/user-guide/installation/source/windows.md
• scripts/bundle-windows.ps1
• scripts/compil.sh
• src/ECS/CMakeLists.txt

Additional comments (Suppressed): 28

scripts/install-deps-windows.ps1 (1)

• 3-20: The new hunks introduce an interactive mode for installing LLVM and CMake if the --interactive argument is passed to the script. This is a good addition as it allows users to manually intervene during the installation process if necessary. However, it's important to ensure that this change is documented in the script's usage instructions so that users are aware of this new feature.

scripts/bundle-macos.sh (1)

• 8-8: The new flags -DPACKAGED=1 -DNO_USE_CLANG_TIDY=true have been added to the cmake command. Ensure that these flags are properly handled in the CMakeLists.txt files and that they have the intended effect on the build process. The -DPACKAGED=1 flag seems to indicate a packaged build, and -DNO_USE_CLANG_TIDY=true appears to disable the use of Clang-Tidy. Verify that these assumptions are correct.

scripts/bundle-linux.sh (1)

• 8-8: The new flags -DPACKAGED=1 -DNO_USE_CLANG_TIDY=true have been added to the cmake command. Ensure that these flags are properly handled in the CMakeLists.txt files and that they have the intended effect on the build process. The -DPACKAGED=1 flag seems to be related to enabling packaging, and -DNO_USE_CLANG_TIDY=true appears to disable the use of Clang-Tidy. Verify that these changes are intentional and correctly implemented.

src/ECS/ResourcesManager.hpp (1)

• 1-16: The ResourcesManager class has been introduced with two static methods convertPath and init. The convertPath method is used to convert a given path by appending it to the _resourcePath. The init method is used to initialize the _resourcePath based on the execPath parameter. The _resourcePath is a static private member of the ResourcesManager class.

The use of a static private member variable _resourcePath might lead to issues if multiple threads are trying to access or modify it at the same time. Consider using a mutex or other synchronization mechanism to prevent data races if this class is intended to be used in a multithreaded context.

Also, the init method seems to be a one-time setup function. If the execPath can change during the runtime of the application, it would be better to handle that scenario as well.

Lastly, the convertPath method takes a const reference to a string but returns a new string. If the conversion is not complex, consider returning a const reference to avoid unnecessary copies.

.github/workflows/release.yml (1)

• 163-168: The new hunk has added the --recurse-submodules flag to the git clone command. This flag is used to clone not only the raylib repository but also its submodules. This change implies that the raylib repository has submodules that are necessary for the build process. Please ensure that the submodules do not introduce any unwanted dependencies or security vulnerabilities.

-        git clone https://github.com/raysan5/raylib.git raylib-repo
+        git clone --recurse-submodules https://github.com/raysan5/raylib.git raylib-repo

src/main_client.cpp (2)

• 25-31: The ResourcesManager.hpp header file has been included. This is in line with the changes mentioned in the PR summary.

• 58-64: The ResourcesManager::init function is called with av[0] as the argument. Ensure that av[0] (which typically contains the path of the executable) is the intended argument for this function. Also, verify that the init function handles potential errors appropriately, as any issues with resource path initialization could impact the entire application.

src/main_server.cpp (2)

• 4-7: The inclusion of the ResourcesManager.hpp header file is new. Ensure that the file exists and is in the correct location.

• 46-52: The ResourcesManager::init function is called with av[0] as the argument. This is typically the execution path of the program. Ensure that this is the intended use and that the init function can handle this input correctly.

src/Client/Raylib/Audio/Audio.hpp (2)

• 37-47: The Sound class now includes a _realPath member variable and a corresponding getRealPath method. Ensure that the _realPath is correctly initialized and updated whenever _path changes. Also, consider the implications of having two separate path variables. If they can get out of sync, it might lead to confusion and bugs.

• 65-75: Similar to the Sound class, the Music class now includes a _realPath member variable and a corresponding getRealPath method. As with the Sound class, ensure that the _realPath is correctly initialized and updated whenever _path changes. Also, consider the implications of having two separate path variables. If they can get out of sync, it might lead to confusion and bugs.

src/ECS/Json.hpp (3)

• 39-39: The isDataExist function has been changed from a non-static to a static function. Ensure that all calls to this function throughout the codebase have been updated to match the new usage.

• 59-59: The constructor of the Json class has been changed from a non-default to a default constructor. This change is in line with the Singleton design pattern, which requires that the constructor be private and default to prevent instantiation from outside the class.

• 73-73: The loadJsonData function has been changed from a non-static to a static function. Ensure that all calls to this function throughout the codebase have been updated to match the new usage.

src/Client/Raylib/Graphics/Graphics.cpp (3)

• 7-13: The ResourcesManager header file is included to use its convertPath function for file path conversion. This change is consistent with the PR summary.

• 380-383: The LoadImage function now uses the ResourcesManager::convertPath function to convert the file path. Ensure that the ResourcesManager has been properly initialized before this point, otherwise, the convertPath function might not work as expected.

• 440-443: The LoadTexture function now uses the ResourcesManager::convertPath function to convert the file path. As with the previous comment, ensure that the ResourcesManager has been properly initialized before this point.

src/Client/Raylib/Audio/Audio.cpp (5)

• 8-9: The inclusion of the ResourcesManager.hpp is a new addition to the file. Ensure that the file exists and is in the correct location.

• 36-42: The Sound constructor now uses the ResourcesManager::convertPath function to convert the file path before loading the sound. This change could affect how file paths are handled throughout the application. Ensure that all file paths passed to the Sound constructor are compatible with the ResourcesManager::convertPath function.

• 106-112: The Sound class now includes a getRealPath method, which returns the converted file path. This is a new addition to the class's interface. Ensure that this method is used correctly throughout the application.

• 116-122: The Music constructor now uses the ResourcesManager::convertPath function to convert the file path before loading the music. This change could affect how file paths are handled throughout the application. Ensure that all file paths passed to the Music constructor are compatible with the ResourcesManager::convertPath function.

• 200-207: The Music class now includes a getRealPath method, which returns the converted file path. This is a new addition to the class's interface. Ensure that this method is used correctly throughout the application.

CMakeLists.txt (2)

• 100-115: The PACKAGED compile definition is being added conditionally based on the PACKAGED variable. Ensure that this variable is being set correctly in all build configurations and that the implications of this definition are well understood throughout the codebase.

• 119-119: No changes have been made in this hunk. The if(MSVC) check is still present as before.

src/ECS/Json.cpp (4)

• 20-20: The loadJsonData function now uses the ResourcesManager::convertPath function to convert the file path. This change is consistent with the PR summary and should help ensure that file paths are handled correctly across different platforms. However, it's important to verify that the ResourcesManager::convertPath function is correctly implemented and that it returns the expected results in all cases.

• 38-38: The indexes variable is now being accessed using const auto &key instead of auto &key. This change is a good practice as it prevents accidental modification of the indexes elements within the loop.

• 83-87: The getDatasFromList function now supports nested arrays, as mentioned in the PR summary. This is a useful feature, but it's important to ensure that it's implemented correctly. In particular, make sure that the recursion doesn't lead to stack overflow errors for deeply nested arrays.

• 150-154: The getInstance method now initializes the _jsonDatas member if it's empty. This is a significant change from the previous implementation, where _jsonDatas was initialized in the constructor. This change could improve performance by delaying the loading of JSON data until it's actually needed. However, it's important to ensure that this change doesn't introduce any race conditions in multithreaded environments.

[coderabbitai]

Review Status

Actionable comments generated: 0

Commits

Files that changed from the base of the PR and between 22638df and 059a330.

Files selected for processing (8)

• .gitignore (1 hunks)
• CMakeLists.txt (3 hunks)
• src/Client/Raylib/Graphics/Graphics.cpp (3 hunks)
• src/ECS/Json.cpp (7 hunks)
• src/ECS/Json.hpp (3 hunks)
• src/ECS/ResourcesManager.cpp (1 hunks)
• src/ECS/ResourcesManager.hpp (1 hunks)
• src/main_server.cpp (2 hunks)

Files skipped from review due to trivial changes (3)

• .gitignore
• src/ECS/ResourcesManager.cpp
• src/ECS/ResourcesManager.hpp

Additional comments (Suppressed): 18

src/main_server.cpp (2)

• 2-8: The inclusion of ResourcesManager.hpp is new. Ensure that the file exists and is in the correct location.

• 46-52: The ECS::ResourcesManager::init(av[0]); line is new. This function initializes the resource path based on the executable path. Ensure that the executable path is always passed as the first argument (av[0]) when the program is run.

src/ECS/Json.hpp (3)

• 41-41: The isDataExist function has been made static. Ensure that all calls to this function throughout the codebase have been updated to match the new usage.

• 61-61: The constructor of Json class has been made private and default. This change enforces the Singleton pattern more strictly by preventing the creation of Json objects using the constructor.

• 75-75: The loadJsonData function has been made static. Ensure that all calls to this function throughout the codebase have been updated to match the new usage.

src/Client/Raylib/Graphics/Graphics.cpp (3)

• 7-13: The ResourcesManager header file is included in the new hunk. This is likely to use the convertPath function from the ResourcesManager class in the ECS namespace. This change is consistent with the PR summary.

• 380-384: The LoadImage function now uses the convertPath function from the ResourcesManager class to convert the file path. This change is consistent with the PR summary. However, ensure that the convertPath function correctly handles all possible path formats that the LoadImage function might receive.

• 439-445: The LoadTexture function now uses the convertPath function from the ResourcesManager class to convert the file path. This change is consistent with the PR summary. However, ensure that the convertPath function correctly handles all possible path formats that the LoadTexture function might receive.

CMakeLists.txt (3)

• 34-40: The destination for the installed assets directory has been changed from "share/${PROJECT_NAME_CLIENT}" to "share/${PROJECT_NAME}". Ensure that this change does not affect the expected location of the assets in the application.

• 61-86: The condition for using Clang-Tidy has been modified. Previously, Clang-Tidy was disabled if NO_USE_CLANG_TIDY was defined and the compiler was MSVC. Now, Clang-Tidy is disabled if NO_USE_CLANG_TIDY is defined, regardless of the compiler. This change makes the use of Clang-Tidy more consistent across different compilers.

• 102-114: A new compile definition PACKAGED has been added for both the client and server targets when PACKAGED is defined. This could be used to conditionally compile code based on whether the application is being packaged or not. Ensure that this new compile definition is used appropriately in the codebase.

src/ECS/Json.cpp (7)

• 20-20: The loadJsonData function now uses the ResourcesManager::convertPath function to convert the path before opening the file. This change is likely to improve the portability of the code by ensuring that the path is always an absolute path based on the initialized resource path. However, it's important to ensure that the ResourcesManager has been properly initialized before this function is called.

• 38-38: The indexes variable is now being iterated over as a const reference, which prevents unnecessary copies and improves performance. This is a good practice when the elements of the container are not being modified.

• 76-76: The list variable is now being iterated over as a const reference, which prevents unnecessary copies and improves performance. This is a good practice when the elements of the container are not being modified.

• 95-95: The list variable is now being iterated over as a const reference, which prevents unnecessary copies and improves performance. This is a good practice when the elements of the container are not being modified.

• 113-113: The list variable is now being iterated over as a const reference, which prevents unnecessary copies and improves performance. This is a good practice when the elements of the container are not being modified.

• 145-145: The indexes variable is now being iterated over as a const reference, which prevents unnecessary copies and improves performance. This is a good practice when the elements of the container are not being modified.

• 164-168: The getInstance function has been modified to load JSON data only if _jsonDatas is empty. This change improves performance by avoiding unnecessary loading of JSON data. However, it's important to ensure that the pathToJson variable is properly initialized before this function is called.