Skip to content

Conversation

@mja00
Copy link
Contributor

@mja00 mja00 commented Oct 9, 2025

Fixes #328

Some mods seem to just place it at the root instead of in the ``[[mods]]` nested object.

@ci010 ci010 requested a review from Copilot October 10, 2025 06:52
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes handling of the clientSideOnly property in Forge mod TOML files by checking for it both at the root level and within nested [[mods]] objects, addressing cases where some mods place this property at the root instead of the expected location.

  • Added clientSideOnly property to the ForgeModTOMLData interface
  • Updated mod parsing logic to check both root and nested mod object for clientSideOnly property

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

loaderVersion: root.loaderVersion as string ?? '',
modLoader: root.modLoader as string ?? '',
issueTrackerURL: root.issueTrackerURL as string ?? '',
clientSideOnly: (root.clientSideOnly as boolean || tomlMod.clientSideOnly as boolean) ?? false,
Copy link

Copilot AI Oct 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logical OR operator will not work as expected here. If root.clientSideOnly is false, the expression will evaluate tomlMod.clientSideOnly, potentially overriding an explicit false value. Use nullish coalescing instead: (root.clientSideOnly as boolean) ?? (tomlMod.clientSideOnly as boolean) ?? false

Suggested change
clientSideOnly: (root.clientSideOnly as boolean || tomlMod.clientSideOnly as boolean) ?? false,
clientSideOnly: (root.clientSideOnly as boolean) ?? (tomlMod.clientSideOnly as boolean) ?? false,

Copilot uses AI. Check for mistakes.
@ci010
Copy link
Collaborator

ci010 commented Oct 10, 2025

Can you also update the test? Or I can merge first and add the test later.

@ci010 ci010 force-pushed the fix/detect-client-only-forge branch from d678f9f to 4c478f6 Compare October 13, 2025 05:27
@ci010 ci010 merged commit 0daaa1d into Voxelum:master Oct 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

clientSideOnly ignored when root in Forge mod's toml

2 participants