Fix client launch missing args - #13
Conversation
Walkthrough
Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (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)
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. Comment |
|
@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: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
worlds/sims4/__init__.py
🧰 Additional context used
🧬 Code graph analysis (1)
worlds/sims4/__init__.py (1)
worlds/sims4/Client.py (1)
main(255-279)
🪛 GitHub Actions: Analyze modified files
worlds/sims4/__init__.py
[error] 22-22: Function is missing a return type annotation [no-untyped-def]
[error] 41-41: Key expression in dictionary comprehension has incompatible type "object"; expected type "str" [misc]
[error] 42-42: Key expression in dictionary comprehension has incompatible type "object"; expected type "str" [misc]
[error] 70-70: Argument 2 to "Sims4Item" has incompatible type "object"; expected "ItemClassification" [arg-type]
[error] 73-73: Function is missing a return type annotation [no-untyped-def]
[error] 83-83: No overload variant of "range" matches argument type "object" [call-overload]
[error] 83-83: No overload variant of "range" matches argument type "object" [call-overload]
[error] 84-84: Argument 1 to "create_item" of "Sims4World" has incompatible type "object"; expected "str" [arg-type]
[error] 95-95: Function is missing a return type annotation [no-untyped-def]
[error] 95-95: Function is missing a type annotation for one or more arguments [no-untyped-def]
⏰ 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). (13)
- GitHub Check: Test Python 3.13 ubuntu-latest
- GitHub Check: Test Python 3.12 ubuntu-latest
- GitHub Check: Test Python 3.11 windows-latest
- GitHub Check: Test Python 3.13 windows-latest
- GitHub Check: Test Python 3.11.2 ubuntu-latest
- GitHub Check: Test Python 3.13 macos-latest
- GitHub Check: Test hosting with 3.13 on ubuntu-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.2 ubuntu-latest
- GitHub Check: Test Python 3.13 macos-latest
- GitHub Check: Test Python 3.11 windows-latest
🔇 Additional comments (1)
worlds/sims4/__init__.py (1)
7-7: Thelaunchfunction properly handles theargsparameter and forwards them correctly to the target function.The launch function signature at line 112 of worlds/LauncherComponents.py accepts
args: Tuple[str, ...]and forwards them correctly in both execution paths:
- When Kivy is running: passes args to
multiprocessing.Process(target=func, args=args), which unpacks and passes them as positional arguments- When Kivy is not running: calls
func(*args)to unpack and pass them directlyNo changes are needed.
Likely an incorrect or invalid review comment.
| def run_client(*args: str): | ||
| from .Client import main | ||
| p = Process(target=main) | ||
| p.start() | ||
| launch(main, name="The Sims 4 Client", args=args) |
There was a problem hiding this comment.
Add missing return type annotation to fix pipeline failure.
The function is missing a return type annotation, which is causing a pipeline failure.
🔎 Proposed fix
-def run_client(*args: str):
+def run_client(*args: str) -> None:
from .Client import main
launch(main, name="The Sims 4 Client", args=args)Verify that arguments are properly passed to the client.
The refactoring from multiprocessing.Process to launch() looks cleaner, but you should verify that the args parameter flows correctly through to the client's argument parser. The main() function in Client.py uses parser.parse_known_args() without explicit arguments, which defaults to parsing sys.argv. Ensure that the launch function properly modifies sys.argv or passes arguments to enable the client to receive the intended command-line arguments.
You can test this by:
- Calling
run_client()with test arguments (e.g.,run_client('--connect', 'localhost:38281')) - Verifying that the client receives and processes these arguments correctly
- Checking debug logs or adding print statements in the client to confirm argument reception
🧰 Tools
🪛 GitHub Actions: Analyze modified files
[error] 22-22: Function is missing a return type annotation [no-untyped-def]
🤖 Prompt for AI Agents
In worlds/sims4/__init__.py around lines 22-24, add an explicit return type
annotation to run_client (it should be -> None) to fix the pipeline failure;
also ensure the client receives the passed arguments by verifying how launch
propagates args — convert the variadic args to an argument list and either (A)
pass that list in the form launch expects so it replaces sys.argv for the child
process or (B) call the client entrypoint with those args directly if launch
does not modify sys.argv; run the suggested manual test (call
run_client('--connect', 'localhost:38281') and confirm parser.parse_known_args()
in Client.py sees the values) and adjust the call to launch or main accordingly.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
worlds/sims4/__init__.py
🧰 Additional context used
🧬 Code graph analysis (1)
worlds/sims4/__init__.py (1)
worlds/sims4/Client.py (1)
main(255-279)
🪛 GitHub Actions: Analyze modified files
worlds/sims4/__init__.py
[error] 11-11: F401 'Items.skills_table' imported but unused
[error] 13-13: F401 'Regions.sims4_skill_dependencies' imported but unused
[error] 13-13: F401 'Regions.sims4_regions' imported but unused
[error] 22-22: E302 expected 2 blank lines, found 1
[error] 31-31: E302 expected 2 blank lines, found 1
[error] 66-66: E303 too many blank lines (2)
[error] 77-77: E501 line too long (123 > 120 characters)
[error] 119-119: E501 line too long (123 > 120 characters)
[error] 132-132: E501 line too long (134 > 120 characters)
[warning] 132-132: W505 doc line too long (134 > 120 characters)
⏰ 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). (10)
- GitHub Check: Test Python 3.12 ubuntu-latest
- GitHub Check: Test Python 3.13 macos-latest
- GitHub Check: Test Python 3.13 windows-latest
- GitHub Check: Test Python 3.11 windows-latest
- GitHub Check: Test Python 3.13 ubuntu-latest
- GitHub Check: Test Python 3.11.2 ubuntu-latest
- GitHub Check: Test Python 3.12 ubuntu-latest
- GitHub Check: Test Python 3.11 windows-latest
- GitHub Check: Test Python 3.13 macos-latest
- GitHub Check: Test Python 3.13 windows-latest
🔇 Additional comments (2)
worlds/sims4/__init__.py (2)
7-7: LGTM - Import addition is correct.The addition of
launchto the imports is necessary for the new implementation and properly structured.
22-22: Return type annotation added successfully.The return type annotation
-> Nonehas been added, addressing the previous pipeline failure.
| from .Version import VERSION, Sims4Version | ||
|
|
||
| def run_client(): | ||
| def run_client(*args: str) -> None: |
There was a problem hiding this comment.
Fix formatting: Add blank lines before function and class definitions.
The pipeline is failing due to PEP 8 violations. Module-level function and class definitions require 2 blank lines before them.
🔎 Proposed fix
Add a blank line before line 22:
from .Version import VERSION, Sims4Version
+
def run_client(*args: str) -> None:
from .Client import main
launch(main, name="The Sims 4 Client", args=args)Add a blank line before line 31:
icon_paths["plumbob"] = f"ap:{__name__}/icons/plumbob.png"
+
class Sims4World(World, UTMixin):
"""
The Sims 4 is the fourth installment in The Sims franchise. Like the previous games in the series,Also applies to: 31-31
🧰 Tools
🪛 GitHub Actions: Analyze modified files
[error] 22-22: E302 expected 2 blank lines, found 1
🤖 Prompt for AI Agents
In worlds/sims4/__init__.py around lines 22 and 31, PEP8 requires two blank
lines before module-level function and class definitions; add one additional
blank line immediately above the def run_client at line 22 and likewise add a
blank line immediately above the class/function at line 31 so there are two
blank lines separating them from previous top-level code/comments.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
worlds/sims4/Client.py
🧰 Additional context used
🪛 GitHub Actions: Analyze modified files
worlds/sims4/Client.py
[error] 255-255: mypy: Function parameters cannot be parenthesized. [syntax]
🪛 Ruff (0.14.10)
worlds/sims4/Client.py
255-255: Expected ), found (
(invalid-syntax)
255-256: Expected an expression
(invalid-syntax)
⏰ 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). (17)
- GitHub Check: build (arm64, ubuntu-24.04-arm, arm64, arm64)
- GitHub Check: build (amd64, ubuntu-latest, amd64, amd64)
- GitHub Check: fuzz
- GitHub Check: Test Python 3.11.2 ubuntu-latest
- GitHub Check: Test Python 3.12 ubuntu-latest
- GitHub Check: Test Python 3.11 windows-latest
- GitHub Check: Test Python 3.13 macos-latest
- GitHub Check: Test Python 3.13 windows-latest
- GitHub Check: Test Python 3.13 ubuntu-latest
- GitHub Check: Test hosting with 3.13 on ubuntu-latest
- GitHub Check: Test Python 3.13 macos-latest
- GitHub Check: Test Python 3.13 windows-latest
- GitHub Check: Test Python 3.11 windows-latest
- GitHub Check: Test Python 3.13 ubuntu-latest
- GitHub Check: Test Python 3.11.2 ubuntu-latest
- GitHub Check: Test Python 3.12 ubuntu-latest
- GitHub Check: Test hosting with 3.13 on ubuntu-latest
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
worlds/sims4/Client.py (1)
255-255: Fix the missing default parameter.The function signature requires an
argsparameter, but line 283 callsmain()without arguments. This will raise aTypeErrorwhen the file is executed directly.🔎 Proposed fix
-def main(args): +def main(args=None):
🧹 Nitpick comments (1)
worlds/sims4/Client.py (1)
258-258: Consider prefixing the unused variable.The
restvariable is unpacked but never used. Consider prefixing it with an underscore to indicate it's intentionally unused.🔎 Proposed fix
- _args, rest = parser.parse_known_args(args) + _args, _rest = parser.parse_known_args(args)
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
.github/workflows/build.ymlis excluded by none and included by none.github/workflows/docker.ymlis excluded by none and included by none
📒 Files selected for processing (1)
worlds/sims4/Client.py
🧰 Additional context used
🪛 Ruff (0.14.10)
worlds/sims4/Client.py
258-258: Unpacked variable rest is never used
Prefix it with an underscore or any other dummy variable pattern
(RUF059)
🔇 Additional comments (1)
worlds/sims4/Client.py (1)
260-260: LGTM!The context initialization correctly uses the parsed
_argsinstead of the shadowedargsvariable. This properly addresses the issue flagged in the previous review.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 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] 102-102: F541 f-string is missing placeholders
[error] 104-104: E501 line too long (168 > 120 characters)
[error] 173-173: F811 redefinition of unused 'logger' from line 11
[error] 178-178: E303 too many blank lines (2)
[error] 192-192: E303 too many blank lines (2)
[error] 200-200: E501 line too long (132 > 120 characters)
[error] 226-226: E501 line too long (122 > 120 characters)
[error] 234-234: E501 line too long (142 > 120 characters)
[error] 246-246: E128 continuation line under-indented for visual indent
🔇 Additional comments (1)
worlds/sims4/Client.py (1)
255-260: LGTM! Argument passing pattern is correct.The function signature and argument handling are now properly implemented. The args parameter is correctly passed to
parse_known_args(args), and the parsed result is used for context initialization. The variable naming (_args) appropriately avoids shadowing the input parameter.
silasary
left a comment
silasary
left a comment
There was a problem hiding this comment.
Code looks good, and looks similar to what I've done elsewhere.
I have not got a chance to test it.
2 participants
This PR should fix the client missing the ability to pass args, however I'm not sure at all, so this is being done in the development branch as a PR before it gets ported to the stable APWorld.
Summary by CodeRabbit
New Features
Bug Fixes
Changes
✏️ Tip: You can customize this high-level summary in your review settings.