Conversation
|
I have cleaned up the base python code to the point where I'm satisfied with functionality. Here's what's left to do:
|
|
After some more thinking we have come to the following solution, which splits
|
|
I believe this method would also require environment check clearing to move from post-load (where it is currently). I'm not entirely sure where it would go, but that's not exactly a huge issue |
… up to Implementer
This allows echecks to provide a helpful "probable_cause" message
9087712 to
fbe6524
Compare
|
After some discussion we have decided to not make environment checks block the initialization of modules:
Instead, the new approach will print warnings to chat once all checks have completed. Below is an outline of how this new approach works: Defining an EnvCheck
Using and EnvCheck
All environment check results are cached per-reload, you do not need to implement caching in your environment checks. |
This makes it easier to keep track of pending checks
|
This is now ready for review. We may want to work on the wording of a failed echeck readout in chat still. |
There was a problem hiding this comment.
We'll want to update the tag lists in this file to reflect changes in #1264
base/data/gm4/function/echeck/score_on_non_player_entity.mcfunction
Outdated
Show resolved
Hide resolved
misode
left a comment
There was a problem hiding this comment.
I'm not sure that I'm a big fan of using the term echeck in the code and file paths. I don't think environment_check is that long.
Right now the only example checks are pretty simple.
- What about for example a check that checks the correct minecraft version (or pack format)?
- Will we need separate environment check identifiers for each new version?
- Do we have to keep all the old version checks in the base?
- What happens when there are multiple GM4 base versions present, some of them having environment checks that the others don't have, would everything still work?
My worry is that with this much abstraction around the environment checks it will be difficult to add some of the more complex checks that require multiple ticks or rely on overlays/json resources.
| @@ -0,0 +1,7 @@ | |||
| # Tests if non-player entities can hold scores. We think spigot might have done this at some point, but are not sure. | |||
There was a problem hiding this comment.
After thinking about this again, we should probably just remove this check. I'll get annoying adding this to every module when it doesn't even matter in the current versions.
| @@ -1,4 +1,6 @@ | |||
| data merge storage gm4:log {queue:[],versions:[]} | |||
| data modify storage gm4:log echecks set value [] | |||
There was a problem hiding this comment.
This command can be combined into the above command
| execute unless data storage gm4:log echecks[{result:{passed:0}}] unless data storage gm4:log echecks[{result:{passed:-1}}] run return 0 | ||
|
|
||
| # copy results into queue, KEEP VERSION IN echecks, SO WE CAN INSPECT IT FOR DEBUGGING | ||
| data modify storage gm4:log queue set from storage gm4:log echecks |
There was a problem hiding this comment.
This feels really weird and hacky. The NBT objects in echecks don't have the same shape as what is expected by queue. This could cause subtle bugs in the future.
This command also completely overwrites the queue field. That feels wrong and might cause problems. For example GM4 base log printing is already delayed to wait for at least one player. Environment checks could finish in between and cause the normal logs to be overwritten.
I would suggest adding a separate recursive function that iterates over the echecks list, or alternatively appending to the existing queue and making sure that it runs a second time if necessary.
|
|
||
| # add environment check requests | ||
| echeck_requests: List[str] = [] | ||
| for namespaced_echeck in opts.echecks: |
There was a problem hiding this comment.
Now that environment checks no longer block the module from loading, we should consider whether we really want these checks to be triggered from load, and not from init.
For example: shamir X has a very specific environment check Y. But shamir X requires that metallurgy is present. When it is not, shamir X will run load but not init because it is disabled. I don't think we want that very specific environment check to complain in that case because the module isn't active anyways.
This could also happen when two versions of a library are present and the one that is disabled has an environment check that the one that is active does not have.
This PR adresses #902.
Environment checks (echecks) are specified in the
beet.yamlof a module, as a list underversioning. E.g.:Upon load, each module requests its environment checks to run. In the
post_loadstage,base(gm4) then calls the function tag#gm4:evaluate_echecks. This function tag is used to ensure versioning works for echecks defined in base or libraries.New echecks may be defined by any module or library. To do so, simply write your echeck in
.mcfunctionfiles, keeping in mind, that you must ensure proper caller context (asandat) yourself. All echeck results are cached during a load, so each echeck will only run once. You do not need to implement caching yourself.Once the
.mcfunctioncode for your echecks is ready, add the#gm4:evaluate_echecksfunction tag to your namespace and point it to your echeck. Done.You may now include your echeck in a module, a library, or base by adding it to the corresponding
beet.yaml.This PR includes the
gm4:echeck/non_player_entity_has_scoreandgm4:echeck/non_player_entity_on_teamchecks. Other checks will have to be written in the future.