Version profiles: Handlebars templates and Minecraft 1.20.4#3
Merged
Conversation
Introduce a data-driven compile pipeline: command formats live in versions/<mc>.json as Handlebars templates, predefined math/builtin/tag files live under build_templates/<category>/<version>/, and Python only stitches templates with runtime parameters. Default and only supported Minecraft version is 1.20.4 (pack format 41, functions folder layout, NBT item syntax). Config uses minecraft_version instead of pack_format. Co-authored-by: Carter Stach <SpyC0der77@users.noreply.github.com>
|
Warning Review limit reached
More reviews will be available in 49 minutes and 39 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces a comprehensive template-driven command generation system for Minecraft datapacks, replacing hardcoded commands with version-aware template rendering via pybars handlebars. It adds version profile JSON configuration, text component support for rich Minecraft text, refactors all compiler layers to thread version context, and updates configuration and documentation. ChangesTemplate-Based Datapack Generation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
|
✅ Created PR with unit tests: #4 |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@BugBot review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
minecraft_script/config_utils.py (1)
10-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop formatting JSON via global string replacement.
In
minecraft_script/config_utils.py_write_config(), the JSON text fromjson.dumps(COMMON_CONFIG)is then mutated with.replace(...). This can corrupt string values containing", "(e.g.,"C:/tmp/a, b"), producing invalidconfig.jsonthat will failjson.loads()on startup.Proposed fix
def _write_config() -> None: - json_file_content: str = ( - json.dumps(COMMON_CONFIG) - .replace("{", "{\n\t") - .replace("}", "\n}") - .replace(", ", ",\n\t") - ) - with open(f"{module_folder}/config.json", "wt", encoding="utf-8") as file: - file.write(json_file_content) + json.dump(COMMON_CONFIG, file, indent=2, ensure_ascii=False) + file.write("\n")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minecraft_script/config_utils.py` around lines 10 - 15, The current _write_config function mutates the JSON string from json.dumps(COMMON_CONFIG) using global string replaces which can corrupt values; instead generate pretty-printed JSON using the json library (e.g., json.dumps or json.dump) with the indent argument and without manual .replace calls, and write that output to the config file; update references in _write_config and any callers (e.g., COMMON_CONFIG) to remove the replace chain and use json.dumps(COMMON_CONFIG, indent=4, ensure_ascii=False) or json.dump(COMMON_CONFIG, fp, indent=4, ensure_ascii=False) to safely produce human-readable JSON.
🧹 Nitpick comments (2)
minecraft_script/compiler/compile_interpreter.py (1)
461-487: 💤 Low valueDouble initialization of version context.
init_version_context(datapack_id)is called both inCompiler.__init__(line 18 of compiler.py) and inmcs_compile(line 462). WhenCompiler.build()callsmcs_compile(), the context is initialized twice with the same ID. This is harmless but redundant.If
mcs_compileis intended to be callable standalone (without aCompilerinstance), this duplication is acceptable. Otherwise, consider removing the call frommcs_compilesince the caller (Compiler) already initializes it.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minecraft_script/compiler/compile_interpreter.py` around lines 461 - 487, mcs_compile currently calls init_version_context(datapack_id) redundantly because Compiler.__init__ already initializes the version context; remove the init_version_context(datapack_id) call from the top of mcs_compile so the context is only initialized by the caller (Compiler) and keep mcs_compile as a thin wrapper that delegates to _mcs_compile(ast, functions_dir, datapack_id).minecraft_script/compiler/compile_types.py (1)
168-186: 💤 Low valueConsider adding
strict=Truetozip()for safety.The
zip()on line 172 doesn't specifystrict=True. Ifparameter_namesandargumentshave mismatched lengths, silent truncation will occur. While argument count validation likely happens earlier in the pipeline, addingstrict=Trueprovides defense-in-depth.🛡️ Proposed fix
- for name, argument in zip(self.parameter_names, arguments): + for name, argument in zip(self.parameter_names, arguments, strict=True):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minecraft_script/compiler/compile_types.py` around lines 168 - 186, In the call method, zip(self.parameter_names, arguments) can silently truncate if lengths differ; update that zip call in the call(self, interpreter, arguments, context) method to use zip(..., strict=True) so mismatched parameter/argument lengths raise an error at runtime (referencing parameter_names, arguments, and the call method) to provide defense-in-depth.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contributors.md`:
- Line 7: Replace the non-descriptive link text "HERE" in the list item (the
markdown link in contributors.md) with a descriptive label such as "breaking
changes page" or "Datapack breaking changes" so the link text conveys its
target; update the markdown link text while keeping the same URL
(https://datapack.wiki/wiki/info/breaking-changes) and ensure it satisfies MD059
accessibility requirements.
In `@minecraft_script/common.py`:
- Line 2: Remove the unused top-level import of the symbol "os" from the module
(it's imported but never used); update the module-level imports in common.py by
deleting the "import os" line so there is no unused import left.
In `@minecraft_script/compiler/builtin_functions.py`:
- Around line 302-312: The code uses two different key types for
interpreter.click_item_lookup (it looks up by click_function object but stores
by click_function.name), so change the lookup to use the same string key:
replace the lookup using click_function with click_function.name (i.e., set
click_function_id = interpreter.click_item_lookup.get(click_function.name)) so
the subsequent assignment interpreter.click_item_lookup[click_function.name] and
the add_command/version.render calls remain consistent.
In `@minecraft_script/compiler/compile_interpreter.py`:
- Line 342: The string passed to add_comment on the variable loop_init_cmd is
marked as an f-string but contains no placeholders; remove the unnecessary f
prefix so the call becomes add_comment(loop_init_cmd, "Initialize async while
loop:") to avoid misleading syntax and a tiny runtime cost—look for the
assignment using loop_init_cmd and the add_comment(...) invocation in
compile_interpreter.py.
In `@minecraft_script/compiler/compiler.py`:
- Line 142: Fix the typo in the progress message by locating the print statement
that currently reads "print('\rBuilding builtin-in functions... Done!')" in
compiler.py and change the string to "Built-in" so it reads "built-in" (i.e.,
"print('\rBuilding built-in functions... Done!')"); ensure only the displayed
text is corrected and behavior remains unchanged.
---
Outside diff comments:
In `@minecraft_script/config_utils.py`:
- Around line 10-15: The current _write_config function mutates the JSON string
from json.dumps(COMMON_CONFIG) using global string replaces which can corrupt
values; instead generate pretty-printed JSON using the json library (e.g.,
json.dumps or json.dump) with the indent argument and without manual .replace
calls, and write that output to the config file; update references in
_write_config and any callers (e.g., COMMON_CONFIG) to remove the replace chain
and use json.dumps(COMMON_CONFIG, indent=4, ensure_ascii=False) or
json.dump(COMMON_CONFIG, fp, indent=4, ensure_ascii=False) to safely produce
human-readable JSON.
---
Nitpick comments:
In `@minecraft_script/compiler/compile_interpreter.py`:
- Around line 461-487: mcs_compile currently calls
init_version_context(datapack_id) redundantly because Compiler.__init__ already
initializes the version context; remove the init_version_context(datapack_id)
call from the top of mcs_compile so the context is only initialized by the
caller (Compiler) and keep mcs_compile as a thin wrapper that delegates to
_mcs_compile(ast, functions_dir, datapack_id).
In `@minecraft_script/compiler/compile_types.py`:
- Around line 168-186: In the call method, zip(self.parameter_names, arguments)
can silently truncate if lengths differ; update that zip call in the call(self,
interpreter, arguments, context) method to use zip(..., strict=True) so
mismatched parameter/argument lengths raise an error at runtime (referencing
parameter_names, arguments, and the call method) to provide defense-in-depth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f260f7d7-4bd3-429e-9dba-8cda0fb11a2a
📒 Files selected for processing (32)
contributors.mdminecraft_script/common.pyminecraft_script/compiler/build_templates/builtins/1.20.4/command.mcfunctionminecraft_script/compiler/build_templates/builtins/1.20.4/give_item.mcfunctionminecraft_script/compiler/build_templates/builtins/1.20.4/log.mcfunctionminecraft_script/compiler/build_templates/builtins/1.20.4/set_block.mcfunctionminecraft_script/compiler/build_templates/builtins/give_item.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/add.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/divide.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/equals.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/greater_equals_than.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/greater_than.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/less_equals_than.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/less_than.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/modulus.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/multiply.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/subtract.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/u_add.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/u_not.mcfunctionminecraft_script/compiler/build_templates/math/1.20.4/u_subtract.mcfunctionminecraft_script/compiler/build_templates/tags/1.20.4/block/no_collision.jsonminecraft_script/compiler/builtin_functions.pyminecraft_script/compiler/compile_interpreter.pyminecraft_script/compiler/compile_types.pyminecraft_script/compiler/compiler.pyminecraft_script/config.jsonminecraft_script/config_utils.pyminecraft_script/shell_commands.pyminecraft_script/version_config.pyminecraft_script/versions/1.20.4.jsonminecraft_script/versions/index.jsonrequirements.txt
💤 Files with no reviewable changes (1)
- minecraft_script/compiler/build_templates/builtins/give_item.mcfunction
|
✅ Created PR with unit tests: #5 |
- Descriptive contributors link label (MD059) - Remove unused os import from common.py - Fix click_item_lookup to use function name as key - Remove redundant init_version_context from mcs_compile - Fix built-in progress typo and pointless f-string - Write config with json.dump(indent=4) instead of string replaces - zip(..., strict=True) for function parameter binding Co-authored-by: Carter Stach <SpyC0der77@users.noreply.github.com>
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
minecraft_script/config_utils.py (1)
32-35:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix wrapper arity mismatch for
minecraft_versionvalidation.Line 34 calls wrappers with one argument, but Line 58 defines
config_minecraft_version_check(value, setting). With Line 71’s direct mapping,update_config("minecraft_version", "...")will raiseTypeErrorinstead of validating.💡 Proposed fix
-def config_minecraft_version_check(value: str, setting: str) -> str: +def config_minecraft_version_check(value: str) -> str: try: load_version_profile(value) except FileNotFoundError as error: supported = list_supported_versions() print(error) if supported: print(f"Supported versions: {', '.join(supported)}") exit(-1) return valueAlso applies to: 58-58, 70-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minecraft_script/config_utils.py` around lines 32 - 35, update_config currently calls wrapper functions as value_wrapper_fnc(value) but config_minecraft_version_check is defined as config_minecraft_version_check(value, setting), causing a TypeError; fix by invoking the wrapper with both arguments: replace value_wrapper_fnc(value) with value_wrapper_fnc(value, setting) so functions like config_minecraft_version_check receive the setting name; ensure this change is applied where COMMON_CONFIG is updated (function update_config) and that all wrapper implementations (e.g., config_minecraft_version_check) accept (value, setting) or are adapted accordingly.minecraft_script/compiler/compile_interpreter.py (1)
417-418:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDebug comment is computed but discarded.
Line 417 wraps
setup_commandswith a comment intocommands, but line 418 still passes the originalsetup_commandstoadd_commands, so the entity selector debug comment is lost.🐛 Proposed fix
setup_commands = self.version.render_lines( "control.entity_selector", childStorage=f"mcs_{local_context.uuid}", parentStorage=f"mcs_{context.uuid}", selector=selector, childPath=local_context.mcfunction_name, ) commands = add_comment(setup_commands, f"Entity selector {selector !r}") - self.add_commands(context.mcfunction_name, setup_commands) + self.add_commands(context.mcfunction_name, commands)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minecraft_script/compiler/compile_interpreter.py` around lines 417 - 418, The debug comment created by calling add_comment(setup_commands, f"Entity selector {selector !r}") is assigned to the local variable commands but never used; add_commands(context.mcfunction_name, setup_commands) still passes the original setup_commands, discarding the comment. Fix by passing the commented list (commands) into add_commands instead of setup_commands so the debug comment is preserved (i.e., replace the call to add_commands(context.mcfunction_name, setup_commands) with add_commands(context.mcfunction_name, commands)).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@minecraft_script/compiler/compile_interpreter.py`:
- Around line 417-418: The debug comment created by calling
add_comment(setup_commands, f"Entity selector {selector !r}") is assigned to the
local variable commands but never used; add_commands(context.mcfunction_name,
setup_commands) still passes the original setup_commands, discarding the
comment. Fix by passing the commented list (commands) into add_commands instead
of setup_commands so the debug comment is preserved (i.e., replace the call to
add_commands(context.mcfunction_name, setup_commands) with
add_commands(context.mcfunction_name, commands)).
In `@minecraft_script/config_utils.py`:
- Around line 32-35: update_config currently calls wrapper functions as
value_wrapper_fnc(value) but config_minecraft_version_check is defined as
config_minecraft_version_check(value, setting), causing a TypeError; fix by
invoking the wrapper with both arguments: replace value_wrapper_fnc(value) with
value_wrapper_fnc(value, setting) so functions like
config_minecraft_version_check receive the setting name; ensure this change is
applied where COMMON_CONFIG is updated (function update_config) and that all
wrapper implementations (e.g., config_minecraft_version_check) accept (value,
setting) or are adapted accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 71254cf3-6abc-4f3e-a4bc-58584655df2b
📒 Files selected for processing (8)
contributors.mdminecraft_script/common.pyminecraft_script/compiler/builtin_functions.pyminecraft_script/compiler/compile_interpreter.pyminecraft_script/compiler/compile_types.pyminecraft_script/compiler/compiler.pyminecraft_script/config.jsonminecraft_script/config_utils.py
💤 Files with no reviewable changes (1)
- minecraft_script/common.py
✅ Files skipped from review due to trivial changes (2)
- contributors.md
- minecraft_script/config.json
🚧 Files skipped from review as they are similar to previous changes (3)
- minecraft_script/compiler/builtin_functions.py
- minecraft_script/compiler/compile_types.py
- minecraft_script/compiler/compiler.py
Pass commented commands to add_commands for entity selectors. Invoke config wrappers with (value, setting) and use check functions directly. Co-authored-by: Carter Stach <SpyC0der77@users.noreply.github.com>
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Owner
Author
|
@cursor review |
Use {{{value}}} so string literals are not HTML-escaped.
Use {{selector}} only; lexer already includes leading @.
Co-authored-by: Carter Stach <SpyC0der77@users.noreply.github.com>
Co-authored-by: Carter Stach <SpyC0der77@users.noreply.github.com>
…blies If/while/for, math, raycast, and datapack execute lines are defined under orchestration.commands and composed via template_assemblies. Each command is compiled as its own Handlebars template so runtime params (datapack_id, branch paths, etc.) still resolve correctly. Co-authored-by: Carter Stach <SpyC0der77@users.noreply.github.com>
- Changed the default Minecraft version in config and version index files to 1.21.2. - Refactored logging functionality in built-in functions to support a new command format. - Removed deprecated built-in command templates for version 1.20.4. - Simplified example script by removing unused functions and adding initialization and main functions. Co-authored-by: Carter Stach <SpyC0der77@users.noreply.github.com>
- Added a "Supported Versions" section with the current version 1.21.2 and its development status. - Enhanced installation instructions for clarity and consistency. - Improved formatting of notes and command usage examples for better readability. - Updated GitHub repository link formatting.
…nctions - Introduced a new text component builder for Minecraft rich text, allowing for JSON serialization and various formatting options. - Added built-in functions: `text()`, `tellraw()`, `title()`, and `title_times()` to support sending messages and titles using the new text component format. - Updated the version configuration to include text component serialization details and commands. - Enhanced example scripts to demonstrate the usage of the new text component features. Co-authored-by: Carter Stach <SpyC0der77@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
minecraft_script/compiler/compiler.py (1)
58-63: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winFinish the path-versioning work for the rest of the output tree.
This class now respects version-specific paths for function tags and
clickable_items, but it still hardcodes"math","builtins","code_blocks","user_functions", and"tags"elsewhere. A profile that changes any of those names will render references from templates into one location and copy/generated files into another. Route all output-tree creation and imports throughself.version.paths[...]consistently.Also applies to: 175-188
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minecraft_script/compiler/compiler.py` around lines 58 - 63, The code currently uses self.function_path(self.version.paths["clickable_items"]) but still hardcodes output subdirectories like "math", "builtins", "code_blocks", "user_functions", and "tags" elsewhere; update all places that create directories, open/write files, or assemble import paths to use self.version.paths["math"], self.version.paths["builtins"], self.version.paths["code_blocks"], self.version.paths["user_functions"], and self.version.paths["tags"] instead of literal strings, including the similar block around the later section referenced (lines ~175-188); ensure all calls to function_path, mkdir, and any template-render targets reference the version.paths entries so generated files and template references are routed to the same versioned locations.
🧹 Nitpick comments (1)
minecraft_script/compiler/compiler.py (1)
149-206: ⚡ Quick winAlways clear the global version context.
clear_version_context()only runs on the happy path. Any exception orexit()before the end ofbuild()leaves the singleton context live for the rest of the process, which can leak version/datapack state into later compiles or tests. Wrap the build body intry/finallyso cleanup is guaranteed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@minecraft_script/compiler/compiler.py` around lines 149 - 206, The build() method currently only calls clear_version_context() on the happy path; wrap the main build body in a try/finally so clear_version_context() always runs (even on exceptions or early exits). Concretely: enclose the existing build logic (everything after start_time until just before clear_version_context()) in a try block and move the clear_version_context() call into the finally block so that clear_version_context() always executes; keep re-raising exceptions (or letting them propagate) so behavior doesn't change otherwise. Reference: build(), clear_version_context(), and the existing exit()/FileExistsError handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@minecraft_script/compiler/build_templates/builtins/1.21.2/log.mcfunction`:
- Around line 1-7: The template currently hard-codes the storage as
"minecraft:temp mcs_log", causing global collisions; update log.mcfunction to
accept and use a datapack-scoped storage identifier/path parameters instead of
the hard-coded namespace (e.g., replace occurrences of "minecraft:temp mcs_log"
in the data modify and $tellraw commands with templated params like a storage
namespace and path token used by the generator), and ensure the generator
supplies those tokens when emitting this builtin so each datapack uses its own
storage id rather than the vanilla minecraft:temp mcs_log.
In `@minecraft_script/compiler/builtin_functions.py`:
- Around line 26-46: Add an explicit arity check before the for-loop to prevent
silent truncation: verify if len(args) > max_length and if so raise a clear
exception (or return an error sentinel) stating "log() accepts at most
{max_length} arguments" so callers know extra arguments are invalid; update the
code near max_length/args (the block that builds storage_suffix and calls
version.render("builtin.log", ...)) to bail out early on too-many-arguments
instead of silently dropping them.
In `@minecraft_script/compiler/compile_types.py`:
- Around line 144-159: MCSTextComponent currently only serializes in
save_to_storage_cmd causing generic copy/transfer paths to bypass text-component
materialization; override the type-level transfer method used by the generic
storage-copy flow (the same hook MCSObject's transfer/copy uses — e.g.,
transfer_to_storage/save_to_storage_cmd fallback) in class MCSTextComponent so
serialization happens whenever the generic path moves data into storage:
implement an override on MCSTextComponent that serializes self.component (using
get_text_component_config and serialize_component) and returns the storage-ready
representation, remove any special-case handling in visit_VariableDeclareNode,
and keep clone() behavior unchanged so all list literals, parameter passing, and
reassignments go through the unified type-level transfer.
In `@minecraft_script/compiler/text_component_builtins.py`:
- Around line 96-100: The _method_text and _method_translate helpers currently
replace result.component outright causing previously applied styling to be lost;
instead, after cloning (MCSTextComponent.clone) build the new base payload via
_text_component_from_string or _translate_component_from_string and merge that
payload into the cloned result.component (preserving existing keys like color,
bold, etc.) rather than assigning result.component = new_payload; update both
_method_text and the similar block handling translate (lines ~132-146) to
perform a shallow/appropriate merge of dictionaries/fields so existing styling
and extras are preserved in the chain.
In `@minecraft_script/interpreter/builtin_functions.py`:
- Around line 232-276: The new builtins custom_tellraw, custom_title, and
custom_title_times currently call print(...) unconditionally and leak stdout;
remove those print calls so the functions only validate args and return
RuntimeResult(return_value=MCSNull()) (keep existing argument checks and use of
MCSString/MCSTextComponent/MCSNumber and RuntimeResult/MCSNull as-is). Ensure no
other logging or side-effect remains in custom_tellraw, custom_title, or
custom_title_times.
- Around line 247-259: The custom_title function currently accepts any string
for the mode (args[1]) but should only allow text-bearing modes; update
custom_title to validate that args[1].get_value() is one of "title", "subtitle",
or "actionbar" and raise an MCSTypeError if it is not, keeping the existing type
checks for MCSString and MCSTextComponent; reference the function name
custom_title and the args indices (args[1] for mode) and adjust the error
message to something like "title() mode must be one of: title, subtitle,
actionbar" so incorrect modes (e.g. clear, reset, times or typos) fail fast.
In `@minecraft_script/version_config.py`:
- Around line 291-304: The current render() always emits all template_assemblies
first then the base template last, which breaks cases that need the base
template interleaved; change render() to treat assemblies[key] as an ordered
sequence that may contain either command path strings or a special marker to
insert the base template in-place (e.g., a sentinel value like {"__template__":
true} or a specific token string), iterate that sequence and for each item
either call render_command(command_path, params) for strings or call
_render_template(key, params) when the marker is encountered; keep backward
compatibility by treating the existing list-of-strings form as the old behavior
(if no marker present then behave like current logic) and update references to
render(), render_command, _render_template and the profile keys
"template_assemblies"/"templates" accordingly.
In `@minecraft_script/versions/index.json`:
- Around line 2-3: The index.json file isn't authoritative because the loader in
minecraft_script/version_config.py still globs versions/*.json; update the
loader (the function that currently enumerates versions via glob — e.g., the
version discovery/get_supported_versions routine) to first read and parse
versions/index.json and use its "supported" and "default" fields to determine
available and default versions (with validation that each named version file
exists), falling back to the old glob behavior only if index.json is missing or
invalid; alternatively, if you prefer the index to be non-authoritative, remove
the unsupported "supported"/"default" contract from versions/index.json so it
can't be mistaken for configuration.
In `@readme.md`:
- Around line 32-34: Fix the broken Markdown in the package note and the
`<path>` placeholder by replacing the stray emphasis delimiters with proper
inline code/backticks: wrap the pip package name fragment (the hyphen) and the
Python package name fragment (the underscore) in single backticks and ensure the
`<path>` placeholder is also enclosed in backticks so it renders as code; update
the affected note text (the sentence starting with "Note: The package's name in
pip..." and the two command sections that mention `<path>`) to remove stray
`*`/`**` characters and use consistent backtick quoting so the note and both
command examples render correctly.
---
Outside diff comments:
In `@minecraft_script/compiler/compiler.py`:
- Around line 58-63: The code currently uses
self.function_path(self.version.paths["clickable_items"]) but still hardcodes
output subdirectories like "math", "builtins", "code_blocks", "user_functions",
and "tags" elsewhere; update all places that create directories, open/write
files, or assemble import paths to use self.version.paths["math"],
self.version.paths["builtins"], self.version.paths["code_blocks"],
self.version.paths["user_functions"], and self.version.paths["tags"] instead of
literal strings, including the similar block around the later section referenced
(lines ~175-188); ensure all calls to function_path, mkdir, and any
template-render targets reference the version.paths entries so generated files
and template references are routed to the same versioned locations.
---
Nitpick comments:
In `@minecraft_script/compiler/compiler.py`:
- Around line 149-206: The build() method currently only calls
clear_version_context() on the happy path; wrap the main build body in a
try/finally so clear_version_context() always runs (even on exceptions or early
exits). Concretely: enclose the existing build logic (everything after
start_time until just before clear_version_context()) in a try block and move
the clear_version_context() call into the finally block so that
clear_version_context() always executes; keep re-raising exceptions (or letting
them propagate) so behavior doesn't change otherwise. Reference: build(),
clear_version_context(), and the existing exit()/FileExistsError handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 729fcb73-c942-4619-9e03-e4118e3deb8f
📒 Files selected for processing (36)
.gitignorecontributors.mddocumentation/builtin-functions.mdexamples/test.mcsminecraft_script/compiler/build_templates/builtins/1.21.2/command.mcfunctionminecraft_script/compiler/build_templates/builtins/1.21.2/give_item.mcfunctionminecraft_script/compiler/build_templates/builtins/1.21.2/log.mcfunctionminecraft_script/compiler/build_templates/builtins/1.21.2/set_block.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/add.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/divide.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/equals.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/greater_equals_than.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/greater_than.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/less_equals_than.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/less_than.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/modulus.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/multiply.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/subtract.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/u_add.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/u_not.mcfunctionminecraft_script/compiler/build_templates/math/1.21.2/u_subtract.mcfunctionminecraft_script/compiler/build_templates/tags/1.21.2/block/no_collision.jsonminecraft_script/compiler/builtin_functions.pyminecraft_script/compiler/compile_interpreter.pyminecraft_script/compiler/compile_types.pyminecraft_script/compiler/compiler.pyminecraft_script/compiler/text_component_builtins.pyminecraft_script/config.jsonminecraft_script/config_utils.pyminecraft_script/interpreter/builtin_functions.pyminecraft_script/interpreter/types.pyminecraft_script/text_components.pyminecraft_script/version_config.pyminecraft_script/versions/1.21.2.jsonminecraft_script/versions/index.jsonreadme.md
💤 Files with no reviewable changes (1)
- minecraft_script/compiler/build_templates/builtins/1.21.2/give_item.mcfunction
✅ Files skipped from review due to trivial changes (4)
- documentation/builtin-functions.md
- contributors.md
- .gitignore
- minecraft_script/config.json
🚧 Files skipped from review as they are similar to previous changes (1)
- minecraft_script/config_utils.py
Comment on lines
+1
to
+7
| data modify storage minecraft:temp mcs_log set value "" | ||
| data modify storage minecraft:temp mcs_log append value from storage $(s0) $(n0) | ||
| data modify storage minecraft:temp mcs_log append value from storage $(s1) $(n1) | ||
| data modify storage minecraft:temp mcs_log append value from storage $(s2) $(n2) | ||
| data modify storage minecraft:temp mcs_log append value from storage $(s3) $(n3) | ||
| data modify storage minecraft:temp mcs_log append value from storage $(s4) $(n4) | ||
| $tellraw @a [{"nbt":"mcs_log","storage":"minecraft:temp","interpret":true}] |
There was a problem hiding this comment.
Don’t store log state in the global minecraft:temp namespace.
This template hard-codes minecraft:temp mcs_log, so every generated datapack shares the same storage slot. Two packs using this builtin can overwrite each other’s log payloads, and custom data should not live under the vanilla minecraft namespace. Please pass a datapack-scoped storage id/path into the template instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minecraft_script/compiler/build_templates/builtins/1.21.2/log.mcfunction`
around lines 1 - 7, The template currently hard-codes the storage as
"minecraft:temp mcs_log", causing global collisions; update log.mcfunction to
accept and use a datapack-scoped storage identifier/path parameters instead of
the hard-coded namespace (e.g., replace occurrences of "minecraft:temp mcs_log"
in the data modify and $tellraw commands with templated params like a storage
namespace and path token used by the generator), and ensure the generator
supplies those tokens when emitting this builtin so each datapack uses its own
storage id rather than the vanilla minecraft:temp mcs_log.
Comment on lines
+26
to
+46
| max_length = 5 | ||
| commands: list[str] = [] | ||
| values: list[tuple[str, str]] = [] | ||
|
|
||
| for i in range(max_length): | ||
| if i < len(args): | ||
| values.append((args[i].get_storage(), args[i].get_nbt())) | ||
| else: | ||
| empty = MCSString(context) | ||
| commands.append(empty.save_to_storage_cmd('""')) | ||
| values.append((empty.get_storage(), empty.get_nbt())) | ||
|
|
||
| storage_suffix = ( | ||
| " {" | ||
| + ", ".join(f'"s{i}": "{values[i][0]}", "n{i}": "{values[i][1]}"' for i in range(max_length)) | ||
| + "}" | ||
| ) | ||
|
|
||
| return commands, MCSNull(context) | ||
| commands.append(version.render("builtin.log", storageSuffix=storage_suffix)) | ||
|
|
||
| return tuple(commands), MCSNull(context) |
There was a problem hiding this comment.
Reject or render more than five log() arguments.
The templated branch still hard-caps log() at five values, so any additional arguments are silently dropped on profiles that use builtin.log. Please add an explicit arity check here if variable-length rendering is not supported yet; silent truncation makes debugging output misleading.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minecraft_script/compiler/builtin_functions.py` around lines 26 - 46, Add an
explicit arity check before the for-loop to prevent silent truncation: verify if
len(args) > max_length and if so raise a clear exception (or return an error
sentinel) stating "log() accepts at most {max_length} arguments" so callers know
extra arguments are invalid; update the code near max_length/args (the block
that builds storage_suffix and calls version.render("builtin.log", ...)) to bail
out early on too-many-arguments instead of silently dropping them.
Comment on lines
+144
to
+159
| class MCSTextComponent(MCSObject): | ||
| def __init__(self, context, component: dict | None = None): | ||
| super().__init__(context, "text_component") | ||
| self.component = dict(component or {}) | ||
| from .text_component_builtins import attach_text_component_methods | ||
|
|
||
| attach_text_component_methods(self) | ||
|
|
||
| def clone(self) -> "MCSTextComponent": | ||
| cloned = MCSTextComponent(self.context, deepcopy(self.component)) | ||
| return cloned | ||
|
|
||
| def save_to_storage_cmd(self, value: any = None) -> str: | ||
| config = get_text_component_config(get_version_context().orchestration) | ||
| serialized = serialize_component(self.component, config) | ||
| return super().save_to_storage_cmd(repr(serialized)) |
There was a problem hiding this comment.
Text components still bypass the generic storage-copy path.
MCSTextComponent only materializes itself when a caller explicitly invokes save_to_storage_cmd(). Everywhere else still uses the inherited MCSObject copy semantics, so list literals, parameter passing, and plain reassignments can read from an uninitialized storage slot. The special-case in visit_VariableDeclareNode is a symptom of that. Please move text-component serialization into the type-level transfer path instead of fixing declarations one call site at a time.
🧰 Tools
🪛 Ruff (0.15.15)
[warning] 156-156: PEP 484 prohibits implicit Optional
Convert to T | None
(RUF013)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minecraft_script/compiler/compile_types.py` around lines 144 - 159,
MCSTextComponent currently only serializes in save_to_storage_cmd causing
generic copy/transfer paths to bypass text-component materialization; override
the type-level transfer method used by the generic storage-copy flow (the same
hook MCSObject's transfer/copy uses — e.g.,
transfer_to_storage/save_to_storage_cmd fallback) in class MCSTextComponent so
serialization happens whenever the generic path moves data into storage:
implement an override on MCSTextComponent that serializes self.component (using
get_text_component_config and serialize_component) and returns the storage-ready
representation, remove any special-case handling in visit_VariableDeclareNode,
and keep clone() behavior unchanged so all list literals, parameter passing, and
reassignments go through the unified type-level transfer.
Comment on lines
+96
to
+100
| def _method_text(receiver: MCSTextComponent, args, context) -> MCSTextComponent: | ||
| string_arg = _require_string_arg(args, "text") | ||
| result = receiver.clone() | ||
| result.component = _text_component_from_string(string_arg) | ||
| return result |
There was a problem hiding this comment.
Preserve existing fields when .text() or .translate() is called later in the chain.
Both helpers replace result.component outright, so chains like text().color("gold").text("Hello") or text().bold().translate("...") lose the styling that was already applied. Merge the new base payload into the cloned component instead of overwriting it.
Suggested fix
def _method_text(receiver: MCSTextComponent, args, context) -> MCSTextComponent:
string_arg = _require_string_arg(args, "text")
result = receiver.clone()
- result.component = _text_component_from_string(string_arg)
+ result.component = {
+ **result.component,
+ **_text_component_from_string(string_arg),
+ }
return result
@@
def _method_translate(receiver: MCSTextComponent, args, context) -> MCSTextComponent:
@@
result = receiver.clone()
- result.component = {
- "translate": _require_literal_string(key_arg, "TextComponent.translate()"),
- }
+ result.component = {
+ **result.component,
+ "translate": _require_literal_string(key_arg, "TextComponent.translate()"),
+ }
return resultAlso applies to: 132-146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minecraft_script/compiler/text_component_builtins.py` around lines 96 - 100,
The _method_text and _method_translate helpers currently replace
result.component outright causing previously applied styling to be lost;
instead, after cloning (MCSTextComponent.clone) build the new base payload via
_text_component_from_string or _translate_component_from_string and merge that
payload into the cloned result.component (preserving existing keys like color,
bold, etc.) rather than assigning result.component = new_payload; update both
_method_text and the similar block handling translate (lines ~132-146) to
perform a shallow/appropriate merge of dictionaries/fields so existing styling
and extras are preserved in the chain.
Comment on lines
+232
to
+276
| def custom_tellraw(args, context): | ||
| from .interpreter import RuntimeResult | ||
| from .types import MCSTextComponent | ||
|
|
||
| if len(args) != 2: | ||
| raise MCSTypeError(f"Function <builtin-tellraw> takes 2 arguments, got {len(args)}") | ||
| if not isinstance(args[0], MCSString): | ||
| raise MCSTypeError("tellraw() target must be a string") | ||
| if not isinstance(args[1], MCSTextComponent): | ||
| raise MCSTypeError("tellraw() message must be a TextComponent") | ||
|
|
||
| print(f"tellraw {args[0].get_value()} {args[1].get_value()}") | ||
| return RuntimeResult(return_value=MCSNull()) | ||
|
|
||
|
|
||
| def custom_title(args, context): | ||
| from .interpreter import RuntimeResult | ||
| from .types import MCSTextComponent | ||
|
|
||
| if len(args) != 3: | ||
| raise MCSTypeError(f"Function <builtin-title> takes 3 arguments, got {len(args)}") | ||
| if not isinstance(args[0], MCSString) or not isinstance(args[1], MCSString): | ||
| raise MCSTypeError("title() target and mode must be strings") | ||
| if not isinstance(args[2], MCSTextComponent): | ||
| raise MCSTypeError("title() message must be a TextComponent") | ||
|
|
||
| print(f"title {args[0].get_value()} {args[1].get_value()} {args[2].get_value()}") | ||
| return RuntimeResult(return_value=MCSNull()) | ||
|
|
||
|
|
||
| def custom_title_times(args, context): | ||
| from .interpreter import RuntimeResult | ||
|
|
||
| if len(args) != 4: | ||
| raise MCSTypeError(f"Function <builtin-title_times> takes 4 arguments, got {len(args)}") | ||
| if not isinstance(args[0], MCSString): | ||
| raise MCSTypeError("title_times() target must be a string") | ||
| if not all(isinstance(value, MCSNumber) for value in args[1:4]): | ||
| raise MCSTypeError("title_times() tick values must be numbers") | ||
|
|
||
| print( | ||
| f"title {args[0].get_value()} times " | ||
| f"{args[1].get_value()} {args[2].get_value()} {args[3].get_value()}" | ||
| ) | ||
| return RuntimeResult(return_value=MCSNull()) |
There was a problem hiding this comment.
Remove the unconditional stdout writes from the new text builtins.
These helpers currently print(...) every call, which changes interpreter output for any script that uses tellraw or title. That looks like leftover debugging rather than program semantics, and it will leak into tests and CLI consumers even though the functions otherwise just return MCSNull().
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minecraft_script/interpreter/builtin_functions.py` around lines 232 - 276,
The new builtins custom_tellraw, custom_title, and custom_title_times currently
call print(...) unconditionally and leak stdout; remove those print calls so the
functions only validate args and return RuntimeResult(return_value=MCSNull())
(keep existing argument checks and use of MCSString/MCSTextComponent/MCSNumber
and RuntimeResult/MCSNull as-is). Ensure no other logging or side-effect remains
in custom_tellraw, custom_title, or custom_title_times.
Comment on lines
+247
to
+259
| def custom_title(args, context): | ||
| from .interpreter import RuntimeResult | ||
| from .types import MCSTextComponent | ||
|
|
||
| if len(args) != 3: | ||
| raise MCSTypeError(f"Function <builtin-title> takes 3 arguments, got {len(args)}") | ||
| if not isinstance(args[0], MCSString) or not isinstance(args[1], MCSString): | ||
| raise MCSTypeError("title() target and mode must be strings") | ||
| if not isinstance(args[2], MCSTextComponent): | ||
| raise MCSTypeError("title() message must be a TextComponent") | ||
|
|
||
| print(f"title {args[0].get_value()} {args[1].get_value()} {args[2].get_value()}") | ||
| return RuntimeResult(return_value=MCSNull()) |
There was a problem hiding this comment.
Validate title() mode before accepting the call.
This overload requires a text component, so only text-bearing modes should be accepted here. Right now any string passes, which means typos and modes like clear, reset, or times are only rejected much later by Minecraft instead of failing fast in the interpreter.
Suggested fix
def custom_title(args, context):
from .interpreter import RuntimeResult
from .types import MCSTextComponent
if len(args) != 3:
raise MCSTypeError(f"Function <builtin-title> takes 3 arguments, got {len(args)}")
if not isinstance(args[0], MCSString) or not isinstance(args[1], MCSString):
raise MCSTypeError("title() target and mode must be strings")
if not isinstance(args[2], MCSTextComponent):
raise MCSTypeError("title() message must be a TextComponent")
+ if args[1].get_value() not in {"title", "subtitle", "actionbar"}:
+ raise MCSTypeError("title() mode must be 'title', 'subtitle', or 'actionbar'")
print(f"title {args[0].get_value()} {args[1].get_value()} {args[2].get_value()}")
return RuntimeResult(return_value=MCSNull())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minecraft_script/interpreter/builtin_functions.py` around lines 247 - 259,
The custom_title function currently accepts any string for the mode (args[1])
but should only allow text-bearing modes; update custom_title to validate that
args[1].get_value() is one of "title", "subtitle", or "actionbar" and raise an
MCSTypeError if it is not, keeping the existing type checks for MCSString and
MCSTextComponent; reference the function name custom_title and the args indices
(args[1] for mode) and adjust the error message to something like "title() mode
must be one of: title, subtitle, actionbar" so incorrect modes (e.g. clear,
reset, times or typos) fail fast.
Comment on lines
+291
to
+304
| def render(self, key: str, **params) -> str: | ||
| parts: list[str] = [] | ||
| assemblies = self.profile.get("template_assemblies", {}) | ||
| if key in assemblies: | ||
| for command_path in assemblies[key]: | ||
| parts.append(self.render_command(command_path, params)) | ||
| templates = self.profile.get("templates", {}) | ||
| if key in templates: | ||
| parts.append(self._render_template(key, params)) | ||
| if not parts: | ||
| raise KeyError( | ||
| f"Template {key!r} not defined for Minecraft {self.profile.get('minecraft_version')!r}" | ||
| ) | ||
| return "\n".join(parts) |
There was a problem hiding this comment.
template_assemblies cannot express the command order several profiles need.
render() always emits every assembly fragment first and the base template last. That breaks assemblies that need pre/post wrapping or interleaving, e.g. math.binary needs store_binary_result after function ...:math/{{operation}}, and control.for.init needs its reset commands after the loop function, not before it. The current renderer will generate invalid mcfunction order for those cases; the assembly format needs an explicit ordered sequence that can place the base template at arbitrary positions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minecraft_script/version_config.py` around lines 291 - 304, The current
render() always emits all template_assemblies first then the base template last,
which breaks cases that need the base template interleaved; change render() to
treat assemblies[key] as an ordered sequence that may contain either command
path strings or a special marker to insert the base template in-place (e.g., a
sentinel value like {"__template__": true} or a specific token string), iterate
that sequence and for each item either call render_command(command_path, params)
for strings or call _render_template(key, params) when the marker is
encountered; keep backward compatibility by treating the existing
list-of-strings form as the old behavior (if no marker present then behave like
current logic) and update references to render(), render_command,
_render_template and the profile keys "template_assemblies"/"templates"
accordingly.
Comment on lines
+2
to
+3
| "default": "1.21.2", | ||
| "supported": ["1.21.2"] |
There was a problem hiding this comment.
versions/index.json is not authoritative yet.
minecraft_script/version_config.py still discovers supported versions by globbing versions/*.json, so changing supported here does not actually restrict what users can select. As written, an unfinished profile file dropped into the directory becomes available even if this index excludes it. Either make the loader read this file or remove the unsupported contract from the index.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@minecraft_script/versions/index.json` around lines 2 - 3, The index.json file
isn't authoritative because the loader in minecraft_script/version_config.py
still globs versions/*.json; update the loader (the function that currently
enumerates versions via glob — e.g., the version
discovery/get_supported_versions routine) to first read and parse
versions/index.json and use its "supported" and "default" fields to determine
available and default versions (with validation that each named version file
exists), falling back to the old glob behavior only if index.json is missing or
invalid; alternatively, if you prefer the index to be non-authoritative, remove
the unsupported "supported"/"default" contract from versions/index.json so it
can't be mistaken for configuration.
Comment on lines
+32
to
34
| *Note: The package's name in pip is written with a hyphen `-`, | ||
| whilst the actual Python package is written with an underscore ``*``._ | ||
|
|
There was a problem hiding this comment.
Fix the broken inline markdown around the package note and <path> placeholder.
The current delimiters render as stray ** / * text, so the pip-package note is garbled and the <path> explanation disappears from both command sections.
Suggested fix
-*Note: The package's name in pip is written with a hyphen `-`,
-whilst the actual Python package is written with an underscore ``*``._
+*Note: The package name on pip uses a hyphen (`-`),
+while the Python package name uses an underscore (`_`).*
@@
-*where ** is a relative or absolute path to your mcs file*
+*where `<path>` is a relative or absolute path to your `.mcs` file*
@@
-*where ** is a relative or absolute path to your mcs file*
+*where `<path>` is a relative or absolute path to your `.mcs` file*Also applies to: 46-57
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@readme.md` around lines 32 - 34, Fix the broken Markdown in the package note
and the `<path>` placeholder by replacing the stray emphasis delimiters with
proper inline code/backticks: wrap the pip package name fragment (the hyphen)
and the Python package name fragment (the underscore) in single backticks and
ensure the `<path>` placeholder is also enclosed in backticks so it renders as
code; update the affected note text (the sentence starting with "Note: The
package's name in pip..." and the two command sections that mention `<path>`) to
remove stray `*`/`**` characters and use consistent backtick quoting so the note
and both command examples render correctly.
- Clarified the behavior of click events in text components, specifying that `click_run` cannot use chat commands like `/say` or `/tell`. - Updated example usage to reflect the correct command for running as the player, enhancing clarity for users.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Other Observations (not in diff)No additional observations outside the diff. Files Reviewed (14 files)
Fix these issues in Kilo Cloud Reviewed by step-3.7-flash-20260528 · 1,034,843 tokens |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/tellraw_rich_text.mcs`:
- Around line 19-34: The comment claiming "each on its own line via append" is
incorrect because the chain currently appends only space separators; update the
example so each link is actually on its own line by replacing the space
separators with newline content (e.g., use append(text().text("\n")) or
append(text().text("\n").color("white")) between entries) or alternatively
change the comment to state they are separated by spaces; locate the variable
links and the append(...) calls on the chain to make this change (references:
links, text(), append(), click_run(), click_suggest(), click_open_url(),
click_copy()).
- Around line 21-22: Replace the unsupported chat-command form passed to
click_run by using the "run as player" form without the leading slash: change
the .click_run("/give `@s` minecraft:emerald 1") call to pass the player-run
command string (e.g., .click_run("give `@s` minecraft:emerald 1")), keeping the
accompanying .hover_text(...) unchanged so the example demonstrates the
supported click_run behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb3d1922-b7c0-47bf-9ca5-7ff917222504
📒 Files selected for processing (2)
documentation/builtin-functions.mdexamples/tellraw_rich_text.mcs
✅ Files skipped from review due to trivial changes (1)
- documentation/builtin-functions.md
SpyC0der77
force-pushed
the
cursor/version-profiles-4e1c
branch
from
35c3d53 to
bfad031
Compare
This was referenced Jun 5, 2026
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the version-profile architecture discussed in the issue thread:
minecraft_script/versions/<version>.jsondefine every stitched mcfunction command (ifs, loops, literals, builtins setup, datapack shell, etc.).compiler/build_templates/{math,builtins,tags}/<version>/(e.g. a distinctadd.mcfunctionper version).Default version
Only Minecraft 1.20.4 is supported for now:
41functions/(not 1.21+function/)SelectedItem.tag.mcs_click,give ...{tag:{mcs_click:...}})Config change
config.jsonusesminecraft_versioninstead ofpack_format.config set minecraft_version 1.20.4selects the profile (validated againstversions/).Dependencies
pybars3inrequirements.txt.Adding future versions
versions/1.20.4.jsonand edit templates.build_templates/*/1.20.4/trees to a new version folder.versions/index.json.See updated
contributors.md.Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores