Linux support #3
Conversation
- Resolved broken dependencies - Linked ResourceLib binaries - Added game detection for Steam on Linux This commit has not been tested on Windows systems, let's hope that still works...
No need to remove these (anymore) as they will just get destroyed
…into linux-moment
- Started detecting more steam installs through the ~/.steam folder - Started detecting games installed through Legendary
This reverts commit 5c9768d.
WalkthroughThe changes introduce a new GitHub Action for automating version updates across multiple configuration files, restructure CI/CD workflows to support multi-platform builds, and enhance resource management in the Tauri application through improved file handling and modularisation. Additionally, modifications to game detection and file handling ensure better cross-platform compatibility. The updates focus on improving automation, compatibility, and resource management. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (11)
.yarn/install-state.gzis excluded by!**/.yarn/**.yarn/releases/yarn-4.0.2.cjsis excluded by!**/.yarn/**src-tauri/Cargo.lockis excluded by!**/*.locksrc-tauri/ResourceLib/ResourceLib-linux-x64/ResourceLib_HM2.sois excluded by!**/*.sosrc-tauri/ResourceLib/ResourceLib-linux-x64/ResourceLib_HM2016.sois excluded by!**/*.sosrc-tauri/ResourceLib/ResourceLib-linux-x64/ResourceLib_HM3.sois excluded by!**/*.sosrc-tauri/ResourceLib/ResourceLib-linux-x64/ResourceLib_HMA.sois excluded by!**/*.sosrc-tauri/ResourceLib/ResourceLib-win-x64/ResourceLib_HM2.dllis excluded by!**/*.dllsrc-tauri/ResourceLib/ResourceLib-win-x64/ResourceLib_HM2016.dllis excluded by!**/*.dllsrc-tauri/ResourceLib/ResourceLib-win-x64/ResourceLib_HM3.dllis excluded by!**/*.dllsrc-tauri/ResourceLib/ResourceLib-win-x64/ResourceLib_HMA.dllis excluded by!**/*.dll
Files selected for processing (20)
- .github/actions/update-versions/action.yml (1 hunks)
- .github/workflows/build.yml (1 hunks)
- .github/workflows/release.yml (2 hunks)
- src-tauri/Cargo.toml (2 hunks)
- src-tauri/ResourceLib/ResourceLib-linux-x64/Include/Embind.h (1 hunks)
- src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceConverter.h (1 hunks)
- src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceGenerator.h (1 hunks)
- src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLib.h (1 hunks)
- src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLibCommon.h (1 hunks)
- src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLib_HM2.h (1 hunks)
- src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLib_HM2016.h (1 hunks)
- src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLib_HM3.h (1 hunks)
- src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLib_HMA.h (1 hunks)
- src-tauri/ResourceLib/ResourceLib-linux-x64/LICENSE (1 hunks)
- src-tauri/build.rs (1 hunks)
- src-tauri/src/game_detection.rs (3 hunks)
- src-tauri/src/general.rs (1 hunks)
- src-tauri/src/show_in_folder.rs (2 hunks)
- src-tauri/tauri.conf.json (1 hunks)
- updates.json (1 hunks)
Additional comments not posted (83)
src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLib_HM2.h (6)
1-1: LGTM!The
#pragma oncedirective is correctly used to prevent multiple inclusions of the header file.The code changes are approved.
2-2: LGTM!The empty line improves readability by separating logical sections of the code.
The code changes are approved.
3-3: LGTM!The
#undefdirective is correctly used to ensure thatZHM_TARGETis not previously defined with a different value.The code changes are approved.
4-4: LGTM!The
#definedirective is correctly used to defineZHM_TARGETwith a value of 2.The code changes are approved.
5-5: LGTM!The empty line improves readability by separating logical sections of the code.
The code changes are approved.
6-6: LGTM!The
#includedirective is correctly used to include the contents ofResourceLib.h.The code changes are approved.
src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLib_HM3.h (6)
1-1: LGTM!The
#pragma oncedirective is correctly used to prevent multiple inclusions of the header file.The code changes are approved.
2-2: LGTM!The empty line improves readability by separating logical sections of the code.
The code changes are approved.
3-3: LGTM!The
#undefdirective is correctly used to ensure thatZHM_TARGETis not previously defined with a different value.The code changes are approved.
4-4: LGTM!The
#definedirective is correctly used to defineZHM_TARGETwith a value of 3.The code changes are approved.
5-5: LGTM!The empty line improves readability by separating logical sections of the code.
The code changes are approved.
6-6: LGTM!The
#includedirective is correctly used to include the contents ofResourceLib.h.The code changes are approved.
src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLib_HM2016.h (6)
1-1: LGTM!The
#pragma oncedirective is correctly used to prevent multiple inclusions of the header file.The code changes are approved.
2-2: LGTM!The empty line improves readability by separating logical sections of the code.
The code changes are approved.
3-3: LGTM!The
#undefdirective is correctly used to ensure thatZHM_TARGETis not previously defined with a different value.The code changes are approved.
4-4: LGTM!The
#definedirective is correctly used to defineZHM_TARGETwith a value of 2016.The code changes are approved.
5-5: LGTM!The empty line improves readability by separating logical sections of the code.
The code changes are approved.
6-6: LGTM!The
#includedirective is correctly used to include the contents ofResourceLib.h.The code changes are approved.
src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLib_HMA.h (4)
1-1: LGTM!The
#pragma oncedirective is correctly used to prevent multiple inclusions.
3-3: LGTM!The
#undef ZHM_TARGETdirective is correctly used to undefine the macro.
4-4: LGTM!The
#define ZHM_TARGET 2012directive is correctly used to define the macro.
6-6: LGTM!The
#include "ResourceLib.h"directive is correctly used to include the necessary header file.updates.json (1)
11-11: LGTM!Adding a newline character at the end of the file is a best practice and is correctly implemented here.
src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLibCommon.h (8)
1-1: LGTM!The
#pragma oncedirective is correctly used to prevent multiple inclusions.
3-3: LGTM!The
#include <cstddef>directive is correctly used to include the necessary header file.
5-8: LGTM!The preprocessor directives correctly ensure compatibility with C++ compilers by using
extern "C"to prevent name mangling.
10-21: LGTM!The
JsonStringstruct is well-defined and the comments provide clear explanations of its members.
23-27: LGTM!The
ResourceMemstruct is well-defined and provides a clear representation of resource data and its size.
29-33: LGTM!The
ResourceTypesArraystruct is well-defined and provides a clear representation of resource types and their count.
35-43: LGTM!The
StringViewstruct is well-defined and the comments provide clear explanations of its members.
45-47: LGTM!The preprocessor directives correctly close the
extern "C"block, ensuring compatibility with C++ compilers..github/actions/update-versions/action.yml (3)
1-2: LGTM!The metadata section is correctly defined.
The code changes are approved.
3-7: LGTM!The inputs section is correctly defined.
The code changes are approved.
8-30: LGTM!The runs section is correctly defined and uses appropriate actions to update the version in various configuration files.
The code changes are approved.
src-tauri/src/show_in_folder.rs (2)
1-3: LGTM!The import section is correctly defined and streamlined.
The code changes are approved.
Line range hint
4-49: LGTM!The function is correctly implemented and the changes improve readability and maintainability.
The code changes are approved.
src-tauri/build.rs (2)
1-28: LGTM!The main function is correctly implemented and the changes improve the build process.
The code changes are approved.
31-43: LGTM!The function is correctly implemented and the changes improve asset management.
The code changes are approved.
.github/workflows/build.yml (7)
8-8: LGTM!The addition of the
workflow_dispatchevent is a good practice for manual triggering of the workflow.The code changes are approved.
11-17: LGTM!The introduction of the matrix strategy for the build job is a great enhancement for multi-platform support.
The code changes are approved.
19-19: LGTM!The dynamic reference to the selected platform from the matrix in the
runs-ondirective is a good improvement.The code changes are approved.
21-21: LGTM!Updating to newer versions of actions is a good practice for maintaining performance, security, and compatibility.
The code changes are approved.
Also applies to: 24-24, 29-29, 34-34, 50-50, 56-56
38-42: LGTM!The conditional step for installing dependencies specific to the Ubuntu platform is a good enhancement for workflow efficiency.
The code changes are approved.
49-53: LGTM!The step for disabling the updater for one-off builds provides greater control over the build process.
The code changes are approved.
55-60: LGTM!The restructuring of the workflow improves its maintainability and clarity.
The code changes are approved.
src-tauri/tauri.conf.json (1)
4-4: Verify the impact of removing the batch script.The simplification of the
beforeBuildCommandto only executeyarn buildmay omit essential tasks previously handled by the batch script. Ensure that any necessary file copying or other tasks are still being performed.src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceConverter.h (6)
1-4: LGTM!Using
#pragma oncefor include guards and including<cstddef>is appropriate.The code changes are approved.
5-8: LGTM!Using
extern "C"to ensure C linkage for the functions is appropriate.The code changes are approved.
Also applies to: 59-61
10-10: LGTM!The forward declaration for
struct JsonStringis appropriate.The code changes are approved.
12-57: LGTM!The
ResourceConverterstruct is well-documented and the function pointers cover various conversion scenarios.The code changes are approved.
52-56: LGTM!The
FreeJsonStringfunction pointer is necessary for proper memory management.The code changes are approved.
14-20: LGTM!The detailed documentation comments are clear and provide useful information about each function pointer.
The code changes are approved.
Also applies to: 22-30, 32-39, 41-50, 53-56
src-tauri/ResourceLib/ResourceLib-linux-x64/Include/Embind.h (5)
12-13: LGTM!The macro definitions are correctly implemented.
The code changes are approved.
29-32: LGTM!The function is correctly implemented.
The code changes are approved.
75-83: LGTM!The function is correctly implemented.
The code changes are approved.
53-73: Add error handling forZHM_TARGET_FUNC(GetGeneratorForResource).The function should check if
ZHM_TARGET_FUNC(GetGeneratorForResource)returns a valid pointer before using it.Apply this diff to add error handling:
val GenerateResourceFromJson(const std::string& p_ResourceType, const std::string& p_ResourceData) { - auto* s_Generator = ZHM_TARGET_FUNC(GetGeneratorForResource)(p_ResourceType.c_str()); + auto* s_Generator = ZHM_TARGET_FUNC(GetGeneratorForResource)(p_ResourceType.c_str()); + if (s_Generator == nullptr) + { + return val::null(); + } if (s_Generator == nullptr) return val::null(); auto* s_Result = s_Generator->FromJsonStringToResourceMem(p_ResourceData.data(), p_ResourceData.size(), false); if (s_Result == nullptr) return val::null(); emscripten::val s_View(emscripten::typed_memory_view(s_Result->DataSize, reinterpret_cast<const uint8_t*>(s_Result->ResourceData))); auto s_JSResult = emscripten::val::global("Uint8Array").new_(s_Result->DataSize); s_JSResult.call<void>("set", s_View); s_Generator->FreeResourceMem(s_Result); return s_JSResult; }Likely invalid or redundant comment.
34-51: Add error handling forZHM_TARGET_FUNC(GetConverterForResource).The function should check if
ZHM_TARGET_FUNC(GetConverterForResource)returns a valid pointer before using it.Apply this diff to add error handling:
val ConvertResourceToJson(const std::string& p_ResourceType, const std::string& p_ResourceData) { - auto* s_Converter = ZHM_TARGET_FUNC(GetConverterForResource)(p_ResourceType.c_str()); + auto* s_Converter = ZHM_TARGET_FUNC(GetConverterForResource)(p_ResourceType.c_str()); + if (s_Converter == nullptr) + { + return val::null(); + } if (s_Converter == nullptr) return val::null(); auto* s_Result = s_Converter->FromMemoryToJsonString(p_ResourceData.data(), p_ResourceData.size()); if (s_Result == nullptr) return val::null(); std::string s_JsonData(s_Result->JsonData, s_Result->StrSize); s_Converter->FreeJsonString(s_Result); return val(s_JsonData); }Likely invalid or redundant comment.
src-tauri/Cargo.toml (2)
64-64: LGTM!The new dependency
home = "0.5.9"is correctly added.The code changes are approved.
139-139: LGTM!The renamed target configuration
[target."cfg(unix)".dependencies]is correctly implemented.The code changes are approved.
src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceGenerator.h (2)
8-8: LGTM!The struct
ResourceMemis correctly declared.The code changes are approved.
10-75: LGTM!The struct
ResourceGeneratoris correctly implemented.The code changes are approved.
src-tauri/ResourceLib/ResourceLib-linux-x64/Include/ResourceLib.h (5)
1-19: LGTM!The includes and macros are correctly defined and necessary for cross-platform compatibility.
The code changes are approved.
21-25: LGTM!The API export/import macros are correctly defined to handle different build configurations.
The code changes are approved.
36-95: LGTM!The function declarations are correctly defined and necessary for the library's functionality.
The code changes are approved.
31-34: LGTM!The extern "C" block is correctly defined to ensure C++ compatibility.
The code changes are approved.
Also applies to: 97-99
101-101: LGTM!The inclusion of Embind.h is necessary for binding C++ code to JavaScript.
The code changes are approved.
.github/workflows/release.yml (3)
9-28: LGTM!The create_release job is correctly defined and necessary for the release workflow.
The code changes are approved.
Line range hint
30-105: LGTM!The build job is correctly defined and necessary for building binaries on multiple platforms.
The code changes are approved.
132-207: LGTM!The post_release job is correctly defined and necessary for updating release information.
The code changes are approved.
src-tauri/ResourceLib/ResourceLib-linux-x64/LICENSE (8)
1-11: LGTM!The preamble is correctly included and necessary for the license.
The code changes are approved.
13-41: LGTM!The additional definitions are correctly included and necessary for the license.
The code changes are approved.
42-45: LGTM!The exception to Section 3 of the GNU GPL is correctly included and necessary for the license.
The code changes are approved.
47-61: LGTM!The terms for conveying modified versions are correctly included and necessary for the license.
The code changes are approved.
63-77: LGTM!The terms for conveying object code are correctly included and necessary for the license.
The code changes are approved.
79-126: LGTM!The terms for conveying combined works are correctly included and necessary for the license.
The code changes are approved.
128-142: LGTM!The terms for conveying combined libraries are correctly included and necessary for the license.
The code changes are approved.
144-165: LGTM!The terms for publishing revised versions are correctly included and necessary for the license.
The code changes are approved.
src-tauri/src/game_detection.rs (4)
1-7: LGTM!The imports and struct definitions are correct and necessary for the functionality of the file.
The code changes are approved.
25-26: LGTM!The
detect_installsfunction is correctly implemented and the modularisation improves code organisation and readability.The code changes are approved.
28-286: LGTM!The
detectionmodule for Windows is correctly implemented and the modularisation improves code organisation and readability.The code changes are approved.
Line range hint
288-429: LGTM!The
detectionmodule for Linux is correctly implemented and the modularisation improves code organisation and readability.The code changes are approved.
src-tauri/src/general.rs (2)
750-764: LGTM!The workaround for case sensitivity issues in file paths on Linux systems is correctly implemented and necessary for ensuring compatibility with the expected folder name on Linux.
The code changes are approved.
766-785: LGTM!The path construction for reading the
packagedefinition.txtfile is correctly implemented and centralises the path handling logic, making it more robust and easier to maintain.The code changes are approved.
- Added even better game detection for Steam - Added support for the Linux version of Hitman 2016 - Set proper permissions for the temp folder on Linux
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (3)
- src-tauri/src/game_detection.rs (4 hunks)
- src-tauri/src/general.rs (1 hunks)
- src-tauri/tauri.conf.json (2 hunks)
Additional comments not posted (4)
src-tauri/tauri.conf.json (2)
4-4: Verify the impact of removing the batch script.The
beforeBuildCommandhas been changed from"yarn build && cmd /c copyThings.bat"to"yarn build". Ensure that the batch script was not performing any critical operations that are now omitted.
18-18: LGTM!The
assetScopehas been expanded to include"$APPDATA/**", which broadens the range of assets the application can utilise.src-tauri/src/game_detection.rs (1)
28-286: LGTM!The introduction of the
detectionmodule for both Windows and Linux enhances code organisation and readability by separating platform-specific implementations.Also applies to: 288-453
src-tauri/src/general.rs (1)
750-773: LGTM!The changes in the
load_game_filesfunction implement a workaround for case sensitivity issues on Linux file systems, ensuring that the correct directory is accessed regardless of the operating system's case sensitivity.
The rfd crate does not work on Linux when combined with Tauri. The dialog needs to run on the event loop, rfd does not do this. [source](https://discord.com/channels/616186924390023171/625037620996734986/842132988318056468)
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
Files selected for processing (3)
- src-tauri/Cargo.toml (3 hunks)
- src-tauri/src/event_handling/resource_overview.rs (26 hunks)
- src-tauri/src/main.rs (6 hunks)
Additional comments not posted (19)
src-tauri/Cargo.toml (2)
63-63: LGTM!The addition of the
homedependency is approved. It will likely enhance the application's capability to manage user-specific configurations or access the home directory.
138-138: LGTM!The renaming of the target configuration from
[target."cfg(linux)".dependencies]to[target."cfg(unix)".dependencies]is approved. This change enhances cross-platform compatibility by encompassing all Unix-like systems.src-tauri/src/main.rs (6)
66-66: LGTM!The import of
FileDialogBuilderfromtauri::api::dialog::blockingis approved. This is necessary for the subsequent changes in the file handling functionality.
448-455: LGTM!The change from
AsyncFileDialogtoFileDialogBuilderfor opening files is approved. This simplifies the file dialog operations by removing the asynchronous nature and directly returning anOption<PathBuf>, improving control flow and readability.
954-971: LGTM!The change from
AsyncFileDialogtoFileDialogBuilderfor saving files is approved. This simplifies the file dialog operations by removing the asynchronous nature and directly returning anOption<PathBuf>, improving control flow and readability.
1139-1156: LGTM!The change from
AsyncFileDialogtoFileDialogBuilderfor saving unlockables patch files is approved. This simplifies the file dialog operations by removing the asynchronous nature and directly returning anOption<PathBuf>, improving control flow and readability.
1190-1196: LGTM!The change from
AsyncFileDialogtoFileDialogBuilderfor general file saving is approved. This simplifies the file dialog operations by removing the asynchronous nature and directly returning anOption<PathBuf>, improving control flow and readability.
1291-1293: LGTM!The change from
AsyncFileDialogtoFileDialogBuilderfor saving specific file types is approved. This simplifies the file dialog operations by removing the asynchronous nature and directly returning anOption<PathBuf>, improving control flow and readability.src-tauri/src/event_handling/resource_overview.rs (11)
18-18: LGTM!The import statement for
FileDialogBuilderis correct and necessary for the changes.
902-922: LGTM! Consider adding context to the errors.The changes are correct and improve the readability and maintainability of the code. However, consider adding context to the errors for better debugging.
if let Some(path) = dialog .set_file_name(&format!("{}.{}", &hash, &file_type)) .add_filter(&format!("{} file", &file_type), &[file_type.as_ref()]) .save_file() { fs::write(&path, data).context("Failed to write data to file")?; fs::write( path .parent() .unwrap() .join(format!("{}.{}.meta", hash, file_type)), metadata_file ).context("Failed to write metadata file")?; }
950-960: LGTM! Consider adding context to the errors.The changes are correct and improve the readability and maintainability of the code. However, consider adding context to the errors for better debugging.
if let Some(path) = dialog .add_filter("QuickEntity entity", &["entity.json"]) .save_file() { fs::write(path, entity_json).context("Failed to write entity JSON to file")?; }
1000-1014: LGTM! Consider adding context to the errors.The changes are correct and improve the readability and maintainability of the code. However, consider adding context to the errors for better debugging.
if let Some(path) = dialog .set_file_name(&format!("{}.TEMP.json", hash)) .add_filter("TEMP.json file", &["TEMP.json"]) .save_file() { fs::write(&path, data).context("Failed to write TEMP data to file")?; fs::write( path .parent() .unwrap() .join(format!("{}.{}.meta.json", hash, metadata_file.hash_resource_type)), to_string(&metadata_file).context("Failed to serialise meta file")? )?; }
1058-1072: LGTM! Consider adding context to the errors.The changes are correct and improve the readability and maintainability of the code. However, consider adding context to the errors for better debugging.
if let Some(path) = dialog .set_file_name(&format!("{}.TBLU", metadata.core_info.id)) .add_filter("TBLU file", &["TBLU"]) .save_file() { fs::write(&path, data).context("Failed to write TBLU data to file")?; fs::write( path .parent() .unwrap() .join(format!("{}.{}.meta", hash, metadata.core_info.resource_type)), metadata_file ).context("Failed to write metadata file")?; }
1133-1147: LGTM! Consider adding context to the errors.The changes are correct and improve the readability and maintainability of the code. However, consider adding context to the errors for better debugging.
if let Some(path) = dialog .set_file_name(&format!("{}.TBLU.json", metadata.core_info.id)) .add_filter("TBLU.json file", &["TBLU.json"]) .save_file() { fs::write(&path, data).context("Failed to write TBLU data to file")?; fs::write( path .parent() .unwrap() .join(format!("{}.{}.meta.json", hash, metadata_file.hash_resource_type)), to_string(&metadata_file).context("Failed to serialise meta file")? )?; }
Line range hint
1174-1198: LGTM! Consider adding context to the errors.The changes are correct and improve the readability and maintainability of the code. However, consider adding context to the errors for better debugging.
if let Some(path) = dialog .set_file_name(&format!("{}.{}.json", hash, res_meta.core_info.resource_type)) .add_filter( &format!("{}.json file", res_meta.core_info.resource_type), &[&format!("{}.json", res_meta.core_info.resource_type)] ) .save_file() { fs::write( &path, to_vec(&convert_generic::<Value>( &res_data, get_loaded_game_version(app, install)?, res_meta.core_info.resource_type )?)? ).context("Failed to write generic RT data to file")?; fs::write( path .parent() .unwrap() .join(format!("{}.{}.meta.json", hash, res_meta.core_info.resource_type)), to_string(&RpkgResourceMeta::from_resource_metadata(res_meta, false)) .context("Failed to serialise meta file")? )?; }
1225-1256: LGTM! Consider adding context to the errors.The changes are correct and improve the readability and maintainability of the code. However, consider adding context to the errors for better debugging.
if let Some(path) = dialog .set_file_name(&format!("{}.json", hash)) .add_filter("JSON file", &["json"]) .save_file() { fs::write(path, res_data).context("Failed to write ORES JSON data to file")?; }
Line range hint
1279-1382: LGTM! Consider adding context to the errors.The changes are correct and improve the readability and maintainability of the code. However, consider adding context to the errors for better debugging.
if let Some(path) = dialog .set_file_name(&format!("{}.png", hash)) .add_filter("PNG file", &["png"]) .add_filter("JPEG file", &["jpg"]) .add_filter("TGA file", &["tga"]) .add_filter("DDS file", &["dds"]) .save_file() { app.track_event( "Extract image file as image format", Some(json!({ "format": path .file_name() .context("No file name")? .to_str() .context("Filename was invalid string")? .split('.') .last() .unwrap_or("None") })) ); match res_meta.core_info.resource_type.as_ref() { "GFXI" => { let reader = ImageReader::new(Cursor::new(res_data.to_owned())).with_guessed_format()?; if path .file_name() .context("No file name")? .to_str() .context("Filename was invalid string")? .ends_with(".dds") { match reader.format().context("Couldn't get format")? { ImageFormat::Dds => { fs::write(path, res_data).context("Failed to write DDS data to file")?; } _ => { send_notification( app, Notification { kind: NotificationKind::Error, title: "DDS encoding not supported".into(), subtitle: "The image is not natively in DDS format and cannot be \ re-encoded as DDS. Please choose another format." .into() } )?; } } } else { reader.decode()?.save(path).context("Failed to save image file")?; } } "TEXT" => { let mut texture = TextureMap::process_data(get_loaded_game_version(app, install)?.into(), res_data) .context("Couldn't process texture data")?; if let Some(texd_depend) = res_meta.core_info.references.first() { let (_, texd_data) = extract_latest_resource(game_files, texd_depend.resource)?; texture .set_mipblock1_data(&texd_data, get_loaded_game_version(app, install)?.into()) .context("Couldn't process TEXD data")?; } if path .file_name() .context("No file name")? .to_str() .context("Filename was invalid string")? .ends_with(".dds") { let dds_data = tex_rs::convert::create_dds(&texture).context("Couldn't convert texture to DDS")?; fs::write(path, dds_data).context("Failed to write DDS data to file")?; } else { let tga_data = tex_rs::convert::create_tga(&texture).context("Couldn't convert texture to TGA")?; let mut reader = ImageReader::new(Cursor::new(tga_data.to_owned())); reader.set_format(image::ImageFormat::Tga); if path .file_name() .context("No file name")? .to_str() .context("Filename was invalid string")? .ends_with(".tga") { fs::write(path, tga_data).context("Failed to write TGA data to file")?; } else { reader.decode()?.save(path).context("Failed to save image file")?; } } } _ => bail!("Unsupported resource type") } }
Line range hint
1406-1431: LGTM! Consider adding context to the errors.The changes are correct and improve the readability and maintainability of the code. However, consider adding context to the errors for better debugging.
if let Some(path) = dialog .set_file_name(&format!("{}.wav", hash)) .add_filter("WAV file", &["wav"]) .save_file() { let (_, res_data) = extract_latest_resource(game_files, hash)?; let data_dir = app.path_resolver().app_data_dir().expect("Couldn't get data dir"); let temp_file_id = Uuid::new_v4(); fs::write(data_dir.join("temp").join(format!("{}.wem", temp_file_id)), res_data).context("Failed to write WEM data to temp file")?; Command::new_sidecar("vgmstream-cli")? .current_dir(data_dir.join("temp")) .args([ &format!("{}.wem", temp_file_id), "-L", "-o", path.to_string_lossy().as_ref() ]) .run() .context("VGMStream command failed")?; }
Line range hint
1452-1513: LGTM! Consider adding context to the errors.The changes are correct and improve the readability and maintainability of the code. However, consider adding context to the errors for better debugging.
if let </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
|
The vgmstream-cli sidecar binary does not have permissions to execute |
Moving the sidecar apparently gives it permission to run.... [source](tauri-apps/tauri#4653 (comment))
|
I fixed the permissions thing. It now uses vgmstream-cli as expected. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files ignored due to path filters (1)
src-tauri/bin/vgmstream-cli-x86_64-pc-windows-msvc.exeis excluded by!**/*.exe
Files selected for processing (1)
- src-tauri/tauri.conf.json (3 hunks)
Additional comments not posted (5)
src-tauri/tauri.conf.json (5)
4-4: Verify the necessity of the removed batch script.The
beforeBuildCommandhas been simplified to justyarn build. Ensure that the removed batch script (cmd /c copyThings.bat) was not performing any essential operations required for the build process.
18-18: LGTM!The expansion of
assetScopeto include"$APPDATA/**"is approved. This change enhances the application's ability to manage user-specific data or configurations.
24-24: LGTM!The change to the
namein theshellconfiguration to"bin/vgmstream-cli"is approved. This update likely aligns with the new path for thevgmstream-clibinary, supporting Linux compatibility.
28-30: LGTM!The additions of
execute,all, andopenproperties in theshellconfiguration are approved. These changes enhance the shell capabilities, allowing for more comprehensive command execution and file handling.
45-45: LGTM!The update to
externalBinto["bin/vgmstream-cli"]is approved. This change likely aligns with the new path for thevgmstream-clibinary, supporting Linux compatibility.
There was a problem hiding this comment.
Actionable comments posted: 7
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (8)
- .github/actions/update-versions/action.yml (1 hunks)
- .github/workflows/build.yml (1 hunks)
- .github/workflows/release.yml (2 hunks)
- src-tauri/build.rs (1 hunks)
- src-tauri/src/event_handling/resource_overview.rs (26 hunks)
- src-tauri/src/game_detection.rs (4 hunks)
- src-tauri/src/general.rs (1 hunks)
- src-tauri/src/main.rs (6 hunks)
Additional comments not posted (10)
.github/actions/update-versions/action.yml (2)
1-7: Metadata and inputs are well-configured.The action's name and description are clear, and the input
versionis properly defined with necessary constraints. Good use of a default value to prevent silent failures.
11-30: Efficient use of community actions for file updates.The steps to update
package.json,Cargo.toml, andtauri.conf.jsonare well-defined using community GitHub Actions. It's recommended to periodically verify the security and maintenance status of these actions to ensure they remain reliable and secure.Consider periodically checking the repositories of
jossef/action-set-json-field@v2andcolathro/[email protected]for updates or security issues..github/workflows/release.yml (3)
Line range hint
28-115: Review thebuildjob configuration.The use of a matrix strategy to accommodate builds on both Ubuntu and Windows platforms is well-implemented. The conditional execution of steps based on the platform is an efficient way to manage dependencies and build processes specific to each environment.
The job configuration is approved.
132-197: Review thepost_releasejob configuration.The configuration for updating the
updates.jsonfile post-release is well-structured. The use of custom actions to manipulate JSON fields ensures that the version tracking and update information are accurately maintained.The job configuration is approved.
9-27: Review thecreate_releasejob configuration.The job configuration for creating a release appears correct. However, consider verifying if
actions/create-release@v1is the latest or most stable version to ensure compatibility and security.The job configuration is approved.
Verification successful
The
create_releasejob is using the latest version of the action.The workflow is correctly configured to use
actions/create-release@v1, which includes the latest versionv1.1.4. No changes are needed regarding the action version.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for the latest version of the create-release action. gh api /repos/actions/create-release/releases/latest --jq .tag_nameLength of output: 76
src-tauri/src/main.rs (1)
451-458: Refactor file dialog operations for better clarity and maintainability.The transition from
AsyncFileDialogtoFileDialogBuildersimplifies the file selection process by removing asynchronous operations. This change should make the code easier to maintain and less prone to errors related to asynchronous handling. However, ensure that all instances where file dialogs are used have been updated to this new method to maintain consistency throughout the application.The changes are approved as they enhance code readability and reduce complexity.
src-tauri/build.rs (1)
2-29: Review of platform-specific linking and function calls inmain().The platform-specific linking logic appears well-structured and uses conditional compilation attributes effectively. However, ensure that the paths specified for both Windows and Linux are correct and accessible on the respective platforms.
The function calls to
copy_static_assets()andtauri_build::build()are appropriate here, but ensure thatcopy_static_assets()handles errors gracefully and logs them appropriately.src-tauri/src/game_detection.rs (1)
Line range hint
1-453: Comprehensive review of game detection restructuring ingame_detection.rs.The restructuring into a
detectionmodule enhances modularity and maintainability, allowing for clearer separation of platform-specific logic. The refactoreddetect_installsfunction appears well-organized, with improved error handling and platform-specific checks.Ensure that all paths and platform-specific conditions are correctly implemented and tested, especially for edge cases and unusual configurations. The use of the
anyhowcrate for error handling is appropriate, but verify that all potential errors are adequately logged and that the error messages provide sufficient context for debugging.Overall, the changes significantly improve the clarity and maintainability of the code. However, thorough testing across all supported platforms is essential to ensure that the detection logic works as expected without introducing new issues.
Verification successful
Verification Successful: Paths and Conditions in
detect_installsare Correctly ImplementedThe code in the
detect_installsfunction uses paths and conditions appropriately, with platform-specific logic and error handling in place. The use ofPath::new,join,exists,is_file, andis_diris consistent with best practices, and the error handling withcontextandbailprovides sufficient context for debugging. No further issues were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all paths and conditions in the `detect_installs` function are correctly implemented. # Test: Simulate game installations and check detection results. Expect: All simulated installations should be detected correctly. echo "Simulating game installations and checking detection results..." # Simulate installations mkdir -p ~/.config/legendary/installed.json echo '{"Eider": {"install_path": "/games/hitman3"}}' > ~/.config/legendary/installed.json # Run detection cargo run --bin detect_gamesLength of output: 467
Script:
#!/bin/bash # Description: Verify the paths and conditions in the `detect_installs` function using `rg` to search for specific patterns. # Search for the use of paths and conditions in the `detect_installs` function rg 'Path::new|join|exists|is_file|is_dir' -A 5 # Search for platform-specific logic and error handling rg 'cfg\(target_os|bail|context|with_context' -A 5Length of output: 366648
src-tauri/src/general.rs (2)
750-762: Address case sensitivity in file paths for Linux compatibility.The added code introduces a workaround for case sensitivity issues on Linux file systems by checking if
relative_runtime_pathis empty and, if not, capitalising the first character. This is a crucial adjustment for ensuring that the application functions correctly across different operating systems where file path case sensitivity varies.The logic to handle case sensitivity is correctly implemented and should resolve potential issues when running on Linux.
764-768: Improve path construction for cross-platform compatibility.The updated code constructs the
runtime_pathby considering both the original and the modified (uppercased)relative_runtime_path. It uses a more robust method to ensure the correct directory is accessed by trying each path variant until a valid one is found. This approach is beneficial for maintaining functionality across different operating systems.The changes enhance the reliability of path handling across platforms, particularly addressing the differences in case sensitivity between Windows and Linux.
There was a problem hiding this comment.
Actionable comments posted: 9
Outside diff range comments (2)
src-tauri/src/main.rs (1)
Line range hint
1193-1296: Refactor file save dialog to handle user cancellation and errors explicitly.The current implementation does not handle the scenario where the user cancels the save operation or if an error occurs during the file dialog operation. This can lead to unhandled exceptions or silent failures, which are not ideal from a usability and error handling perspective.
Here's a suggested refactor to explicitly handle these scenarios:
- if let Some(path) = dialog.save_file() { + match dialog.save_file() { + Some(path) => { // existing code to handle file save + }, + None => { + // Log error and notify user + log::error("File save operation was cancelled or failed."); + send_notification(app, Notification { + kind: NotificationKind::Error, + title: "File Save Error".to_string(), + subtitle: "The file save operation was cancelled or failed.".to_string() + })?; + } + }This change ensures that all scenarios are handled gracefully, providing a better user experience by informing the user of what went wrong and preventing the application from proceeding without a valid file path.
src-tauri/src/game_detection.rs (1)
Line range hint
25-450: Review the restructuring and modularisation indetect_installs.The restructuring of the
detect_installsfunction into a nesteddetectionmodule for different operating systems is a significant improvement in terms of code organisation and maintainability. This modular approach allows for clearer separation of concerns and better management of platform-specific code. Here are some specific points:
- The use of the
anyhowcrate for error handling enhances the clarity and context of error messages, which is crucial for debugging.- The modularisation into
detectionmodules for Windows and Linux ensures that the code is more organised and easier to maintain.- The detailed checks for game installations, including specific game IDs and paths, improve the accuracy and robustness of the detection logic.
However, there are areas that could be further improved:
- The error handling could be more consistent, especially in the use of
contextfor providing detailed error messages.- The code could benefit from further documentation, especially explaining the logic behind specific checks for game IDs and paths.
Enhance error handling and documentation. Here's a proposed change for more consistent error handling:
- serde_json::from_slice(&fs::read(legendary_installed_path).context("Reading legendary installed")?) + serde_json::from_slice(&fs::read(legendary_installed_path).with_context(|| format!("Failed to read installed data from {}", legendary_installed_path.display()))?)And add comments to explain the logic behind specific game ID checks:
// Check for Hitman 3 installation by game ID if folder.apps.contains_key("1659040") { check_paths.push(( PathBuf::from(&folder.path) .join("steamapps") .join("common") .join("HITMAN 3"), "Steam" )); }
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (7)
- src-tauri/build.rs (1 hunks)
- src-tauri/src/event_handling/resource_overview.rs (25 hunks)
- src-tauri/src/game_detection.rs (4 hunks)
- src-tauri/src/general.rs (1 hunks)
- src-tauri/src/intellisense.rs (1 hunks)
- src-tauri/src/main.rs (6 hunks)
- src-tauri/src/show_in_folder.rs (2 hunks)
Additional comments not posted (1)
src-tauri/src/intellisense.rs (1)
475-476: Optimisation of Property Loop HandlingThe modification to directly use the result of
get_cppt_propertiesin the loop without cloning enhances efficiency by reducing unnecessary memory allocation. This change is well-aligned with the PR's goal of optimising the code for better performance on Linux systems.The changes are approved as they improve performance without altering the existing logic.
Linux moment 🥳
Summary by CodeRabbit
New Features
$APPDATAdirectory.Bug Fixes
Chores