Skip to content

Add an additional check that /set_path was given an absolute file path - #22

Merged
silasary merged 3 commits into
sims4-devfrom
client-fix
Jan 11, 2026
Merged

Add an additional check that /set_path was given an absolute file path#22
silasary merged 3 commits into
sims4-devfrom
client-fix

Conversation

@silasary

@silasary silasary commented Jan 11, 2026

Copy link
Copy Markdown

In-client feedback that the user didn't provide the expected input for the command.

Summary by CodeRabbit

  • Bug Fixes
    • Improved path validation with clearer error messages when no path is provided.
    • Added user-facing guidance prompting for full (absolute) paths and examples to reduce input errors.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 11, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Input validation in _cmd_set_path now treats empty input via truthiness, adds a branch to handle non‑absolute paths with guidance/examples, and maintains existing path existence checks. The main() function signature was changed to include explicit type hints and a -> None return annotation.

Changes

Cohort / File(s) Summary
Input Validation & Type Annotations
worlds/sims4/Client.py
_cmd_set_path: improved empty-input check (truthiness), added non-absolute path branch that prompts for a full/absolute path with example guidance, preserved existing existence checks and mod_data_path assignment. main(): signature updated to `def main(args: list[str]

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 I nudged a path to be absolute and neat,
I checked for emptiness with a tiny feat,
Type hints now sparkle, tidy and bright,
The rabbit hops off—code feels light! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description is incomplete. It lacks the required template structure including project targeting, testing information, and detailed explanation of what is being fixed. Follow the repository template: add project context, explain the fix more thoroughly under 'What is this fixing or adding?', and document how this was tested.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding a check to validate that /set_path receives an absolute file path.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch client-fix

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c4912f7 and 09a1875.

📒 Files selected for processing (1)
  • worlds/sims4/Client.py
🧰 Additional context used
🪛 GitHub Actions: Analyze modified files
worlds/sims4/Client.py

[error] 11-11: Module "CommonClient" does not explicitly export attribute "ClientStatus" [attr-defined]


[error] 16-16: Cannot find implementation or library stub for module named "worlds.tracker.TrackerClient" [import-not-found]


[error] 37-37: Function is missing a return type annotation [no-untyped-def]


[error] 49-49: Function is missing a return type annotation [no-untyped-def]


[error] 70-70: Function is missing a return type annotation [no-untyped-def]


[error] 74-74: Function is missing a return type annotation [no-untyped-def]


[error] 77-77: "CommonContext" has no attribute "syncing" [attr-defined]


[error] 78-78: Argument 3 to "print_json" has incompatible type "CommonContext"; expected "SimsContext" [arg-type]


[error] 81-81: Function is missing a return type annotation [no-untyped-def]


[error] 83-83: "CommonContext" has no attribute "goal" [attr-defined]


[error] 86-86: Function is missing a return type annotation [no-untyped-def]


[error] 88-88: "CommonContext" has no attribute "career" [attr-defined]


[error] 91-91: Function is missing a return type annotation [no-untyped-def]


[error] 99-99: Incompatible types in assignment (expression has type "str", variable has type "Path") [assignment]


[error] 109-109: Class cannot subclass "SuperContext" (has type "Any") [misc]


[error] 116-116: Function is missing a type annotation [no-untyped-def]


[error] 123-123: Function is missing a return type annotation [no-untyped-def]


[error] 128-128: Function is missing a return type annotation [no-untyped-def]


[error] 128-128: Missing type parameters for generic type "dict" [type-arg]


[error] 165-165: Tuple index out of range [misc]


[error] 186-186: Cannot determine type of "seed_name" [has-type]


[error] 208-208: Function is missing a return type annotation [no-untyped-def]


[error] 211-211: Function is missing a return type annotation [no-untyped-def]


[error] 218-218: Function is missing a return type annotation [no-untyped-def]


[error] 225-225: Call to untyped function "load_json" in typed context [no-untyped-call]


[error] 249-249: Call to untyped function "load_json" in typed context [no-untyped-call]


[error] 258-258: Function is missing a return type annotation [no-untyped-def]


[error] 258-258: Call to untyped function "SimsContext" in typed context [no-untyped-call]


[error] 262-262: Call to untyped function "_main" in typed context [no-untyped-call]

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
  • GitHub Check: mypy
  • GitHub Check: flake8
  • GitHub Check: fuzz
  • GitHub Check: Test Python 3.13 macos-latest
  • GitHub Check: Test Python 3.13 windows-latest
  • GitHub Check: Test Python 3.11.2 ubuntu-latest
  • GitHub Check: Test Python 3.11 windows-latest
  • GitHub Check: Test Python 3.12 ubuntu-latest
  • GitHub Check: Test Python 3.13 ubuntu-latest
  • GitHub Check: tests
  • GitHub Check: Test hosting with 3.13 on ubuntu-latest
  • GitHub Check: Test Python 3.13 ubuntu-latest
  • GitHub Check: Test Python 3.11.2 ubuntu-latest
  • GitHub Check: Test Python 3.11 windows-latest
  • GitHub Check: Test Python 3.13 windows-latest
  • GitHub Check: Test Python 3.13 macos-latest
  • GitHub Check: Test Python 3.12 ubuntu-latest
  • GitHub Check: Test hosting with 3.13 on ubuntu-latest
🔇 Additional comments (3)
worlds/sims4/Client.py (3)

95-96: LGTM: Improved error message.

The updated message is clearer and fixes the typo in the original text.


97-101: Consider control flow: non-absolute paths can succeed if they exist.

The new absolute path check improves user feedback, but note that the control flow allows relative paths to succeed if the expected directory structure exists (line 97 check precedes line 100). This is probably acceptable since it only shows the "full path" guidance when the path doesn't work.

However, if strict enforcement is desired (per PR objective "ensure the argument is an absolute file path"), consider checking os.path.isabs(p) immediately after the empty-path check, before verifying existence.

Also note: Line 99 has a pre-existing type incompatibility flagged by the pipeline—mod_data_path is typed as Path (line 33) but assigned a str from os.path.join. Consider using Path(...) or declaring mod_data_path as str.


257-257: LGTM: Type hints improve code quality.

The addition of type annotations to main() is a good step toward better type safety. While the pipeline shows many other functions still lack type hints, this change is correct and beneficial.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
worlds/sims4/Client.py (1)

91-106: Critical: Absolute path validation must occur before existence check.

The current logic allows relative paths to be accepted if they happen to exist (line 97-99), which would store a relative path in mod_data_path. This can break subsequent operations when the working directory changes.

The absolute path check at line 100 only catches non-absolute paths that don't exist, but misses relative paths that do exist.

🔧 Proposed fix

Move the absolute path validation before the existence check and fix the typo:

 def _cmd_set_path(self, sims_4_mods_path: str = ''):
     """Set the file path to the Sims 4 mods folder manually (if automatic detection fails)"""
     p = sims_4_mods_path
     global mod_data_path
     if not p:
-            self.output("no path inputed")
+            self.output("No path provided")
+    elif not os.path.isabs(p):
+            self.output("Please enter the full path to the Sims 4 mods folder.\n"
+                        "For example: C:\\Users\\Username\\Documents\\Electronic Arts\\The Sims 4\\Mods")
     elif os.path.exists(os.path.join(p, 'mod_data', 's4ap')):
         self.output('Sims 4 mods folder found')
         mod_data_path = os.path.join(p, 'mod_data', 's4ap')
-    elif not os.path.isabs(p):
-            self.output("Please enter the full path to the Sims 4 mods folder.\nFor example: C:\\Users\\Username\\Documents\\Electronic Arts\\The Sims 4\\Mods")
     else:
         self.ctx.gui_error(title='Sims 4 mods folder not found',
-                               text=f'Make sure the file path you inputed is correct.')
+                               text='Make sure the file path you entered is correct.')
         self.output(
-                f'Could not find mod_data folder\nif the path you inputed is correct make sure you have enabled script mods in the sims 4 and run the game \nPath: {p}')
+                f'Could not find mod_data folder\nif the path you entered is correct make sure you have enabled script mods in the sims 4 and run the game \nPath: {p}')

Note: This also fixes the typo "inputed" → "entered" on lines 104 and 106, and splits the long line 101 for readability.

🤖 Fix all issues with AI agents
In @worlds/sims4/Client.py:
- Around line 95-96: Replace the typo in the user-facing message emitted by
Client when no path is given: locate the call self.output("no path inputed") in
the Client class (method handling path input) and change the string to correct
English, e.g. "No path provided" or "No path inputted", preserving
capitalization and punctuation to match surrounding messages.
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 27aab9f and c4912f7.

📒 Files selected for processing (1)
  • worlds/sims4/Client.py
🧰 Additional context used
🪛 GitHub Actions: Analyze modified files
worlds/sims4/Client.py

[error] 11-11: F401 'CommonClient.logger' imported but unused


[error] 21-21: E402 module level import not at top of file


[error] 25-25: E302 expected 2 blank lines, found 1


[error] 30-30: E305 expected 2 blank lines after class or function definition, found 1


[error] 37-37: E302 expected 2 blank lines, found 1


[error] 41-41: F541 f-string is missing placeholders


[error] 41-41: E501 line too long (126 > 120 characters)


[error] 46-46: E305 expected 2 blank lines after class or function definition, found 1


[error] 76-76: F541 f-string is missing placeholders


[error] 101-101: E501 line too long (160 > 120 characters)


[error] 104-104: F541 f-string is missing placeholders


[error] 106-106: E501 line too long (168 > 120 characters)


[error] 175-175: F811 redefinition of unused 'logger' from line 11


[error] 180-180: E303 too many blank lines (2)


[error] 194-194: E303 too many blank lines (2)


[error] 202-202: E501 line too long (132 > 120 characters)


[error] 228-228: E501 line too long (122 > 120 characters)


[error] 236-236: E501 line too long (142 > 120 characters)


[error] 248-248: E128 continuation line under-indented for visual indent


[error] 248-248: E302 expected 2 blank lines, found 1


[error] 248-248: E303 too many blank lines (2)


[error] 248-248: E305 expected 2 blank lines after class or function definition, found 1


[error] 248-248: E402 module level import not at top of file


[error] 248-248: E501 line too long (126 > 120 characters)


[error] Flake8 reported multiple syntax/style errors


[error] Process completed with exit code 1.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: Test Python 3.11 windows-latest
  • GitHub Check: Test hosting with 3.13 on ubuntu-latest
  • GitHub Check: Test Python 3.11.2 ubuntu-latest
  • GitHub Check: Test Python 3.13 macos-latest
  • GitHub Check: Test Python 3.12 ubuntu-latest
  • GitHub Check: Test Python 3.13 windows-latest
  • GitHub Check: Test Python 3.13 ubuntu-latest
  • GitHub Check: Test Python 3.11 windows-latest
  • GitHub Check: Test Python 3.13 windows-latest
  • GitHub Check: Test Python 3.12 ubuntu-latest
  • GitHub Check: Test Python 3.11.2 ubuntu-latest
  • GitHub Check: Test Python 3.13 ubuntu-latest
  • GitHub Check: Test Python 3.13 macos-latest
  • GitHub Check: Test hosting with 3.13 on ubuntu-latest
🔇 Additional comments (1)
worlds/sims4/Client.py (1)

257-257: Type hints are correct; no Python 3.10+ requirement due to __future__ annotations import.

The type hints improve code clarity and are implemented correctly. The list[str] | None syntax works on Python 3.9+ because from __future__ import annotations is imported at the module level, which postpones annotation evaluation per PEP 563. The parse_known_args(args) call on line 260 correctly accepts the optional list parameter.

Comment thread worlds/sims4/Client.py Outdated
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@silasary
silasary merged commit 355c29f into sims4-dev Jan 11, 2026
25 checks passed
@silasary
silasary deleted the client-fix branch January 11, 2026 01:21
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.

1 participant