Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

High level method suggestions #16

Open
Caellian opened this issue Apr 20, 2020 · 15 comments
Open

High level method suggestions #16

Caellian opened this issue Apr 20, 2020 · 15 comments

Comments

@Caellian
Copy link

Caellian commented Apr 20, 2020 •
edited
Loading

I know some of these might end up being very expensive. Sadly, Mojang is doing a $#!t job at improving their command system to perform better in complex mcfunction files.

Here's my list of suggestions:

Working with inventories

In C++ version, the container could be a class holding context about container position, possibly extra position for double chests so that you don't have to check for adjacency every call. In C it's a struct.

  • get_item_count(container, item, slot_range=all)
    • test every slot in container for each number 1-64, sum them, use result - yes, very slow
  • has_item(container, item, count=1)
    • same as above but terminates earlier so a bit faster
  • modify_item_count(container, item, count=1)
    • positive count values add items, negative remove items
  • Support for item frames.
    • Methods mentioned so far would allow spiffy and compact sorting systems to be written for example.

Working with data

I suggest you allow loading files (just one format is more than enough) and using their content. JSON files would probably be the best choice as MC uses JSON for its data pack content.
Using fopen/fstream would be confusing, I suggest a custom name like json("rel_path.json").
This would for obvious reasons be read-only. Treat object as a struct. During compile-time, read content from the path and bake in all possible permutations (YES, slowness) based on possible states.

Make IDs a primitive type

Store a local list of all blocks and items - generated from decompiled minecraft code, not sure if there's a better source - so that they can be treated as primitive types. This would ease the pain of generating large numbers of permutations for commands which interact with inventories.

Block states

Allow interacting with block states. These can be optimized a bit as they are block dependant. Information can be generated from each version data. Access from C++ with operator[], from C through struct values.

I haven't looked at Minecraft for a very long time so some of these might be possible to do in a better/more optimized way. I'm aware these are a lot, but they would make writing huge mcfunctions files simple as it's easier to generate 10000 lines of commands from python than actually writing them. Having said that, I'll create yet another suggestion on r/minecraftsuggestions and spam the discord that they add commands for populating scoreboards with item counts and reducing/increasing stack sizes in inventories.

I'm currently very busy, but your project is something that I've been thinking about for two years now and I'm very interested in how it pans out. I'd love to contribute, I might sometime soon, but I'm currently working on a decompiler.

Edit 1

Raytracing

Another commonly used function that should work 99% of the time and is extremely useful is raycasting. There are some problems with it (like detecting non-full blocks), but these could be supported via tags (as in block/item lists in data json). Here's a nice tutorial I found for it which explains in great detail several techniques and their drawbacks. A few things have changed since it was made but generally, it still applies. Some things could be optimized a bit for instance.

Possibly even snowballs could be used by teleporting them and changing their velocity (it should be possible I think), this would provide a better performance and a much better collision detection than using armour stands. Would involve complicated calculation for velocity but that's only a problem to do without a script or a compiler.
@chorman0773
Copy link
Contributor

chorman0773 commented Apr 20, 2020 via email •
edited
Loading

@simon816
Copy link
Owner

get_item_count(container, item, slot_range=all)
test every slot in container for each number 1-64, sum them, use result - yes, very slow

It's not actually necessary to test specific quantities directly. You can probe the NBT to obtain the "Count" variable:
image

/execute store result score @s g0_x run data get block -1 56 -5 Items[0].Count
image

It is also possible to apply a filter to an NBT path query directly:

/data modify entity @e[tag=ns_global,limit=1] ArmorItems[0].tag.working.items set value []
/data modify entity @e[tag=ns_global,limit=1] ArmorItems[0].tag.working.items append from block -1 56 -5 Items[{"id": "minecraft:stone", Count:32b}]

This creates an NBT list of items that are minecraft:stone and have a count of 32. This list can then be aggregated (e.g. summed)

/data get entity @e[tag=ns_global,limit=1] ArmorItems[0].tag.working.items
image

In the trivial case, has_item can compile directly into an NBT path query like the above.
It complicates things if we consider runtime (non constant) variables as we can't construct NBT path queries at runtime.

Working with data

I was thinking the datapack definition could allow bundling other files, though that's slightly different.

It's an interesting idea and I can see it being useful. I hadn't considered loading static data from json files.

Make IDs a primitive type

CBL comes with a pre-baked list of items, blocks and entities:
https://github.com/simon816/Command-Block-Assembly/blob/master/cbl/include/Items
https://github.com/simon816/Command-Block-Assembly/blob/master/cbl/include/Blocks
https://github.com/simon816/Command-Block-Assembly/blob/master/cbl/include/Entities

Currently there isn't much possible with these, but I hope to add functions such as Block::with_state to create a blockstate with the given attributes.

As @chorman0773 states it would break across minecraft versions. This is true, ideally I would allow specifying a target version. It is to be expected that datapack authors check whether anything may have broken with a new game version.

The thing I'm working on at the moment is a meta-language for CBL and Command IR to be able to run code at compile time (essentially macros). This will make it easier to generate large permutations of commands without copy+pasting.

@chorman0773
Copy link
Contributor

Baking the list also breaks with modded uses, for which I would actually make use of frequently. I would not recommend baking the ids of items/blocks/entities, as I mentioned, except specifically those present in the minecraft namespace, which you can assume is unmodified between versions.

@simon816
Copy link
Owner

simon816 commented Apr 21, 2020 •
edited
Loading

The baked in items/blocks/entities are for convenience. A modded block will be created like so:
new BlockType("modname:blockname")
(I don't have string-based constructor support currently, so cannot do this at the moment)

@Caellian
Copy link
Author

Caellian commented Apr 28, 2020 •
edited
Loading

Static data doesn't need to necessarily break every version. It is possible to download Minecraft jar for version X by accessing version json launchers download, download version X jar, decompile it (Items.class and Blocks.class or however they're actually called) with fernflower using a custom renamer which loads newly provided (proguard generated) mappings and then extract lists of items and blocks using regex patterns (extract the name from register\((\(String))?"([a-z_]+)") for version X.
This would possibly also allow gathering some other information like potion effects.
Items.java contains actually obtainable block items too, this complicates regex a bit but could also provide more useful information.
If this is being done I suggest the person doing it does it in a separate repository and shares generated config files with the rest of the world.
GitHub also has Actions now which could allow checking whether a new version is available every half a week or so using cron jobs and automatically generating these and then pushing a new commit with updated data. For public projects, there's no imposed time limits for the Actions.
While regex is good enough for lists of blocks and items, using a java parser and some code, one could also bundle in information about blocks like hardness, required tools, drop items, ...
These things rarely change (last big change was when block states were added), so maintenance on something like this would be very low.
I might actually do something along those lines when I'm at least half done with what I'm currently working on.

@chorman0773 Almost nothing can be done to support mods. Above described process could theoretically be applied to mods too but is unlikely to work with all of them.
The inability of generating iterators over blocks for mods should not force this project into not supporting them at all. Supporting just Vanilla is much better than not supporting anything. They are a very useful and time-saving feature. Maybe it's a good idea to warn people about this in the documentation and tell them to handle other blocks/items separately.
If you write a data pack requiring behaviour which warrants usage of block iterators, you won't be able to support mods with or without using CBA so I don't really see your point. This limitation is imposed by Minecraft commands, not the suggested feature. I don't see how it can be implemented any better.
It's also very difficult to support mods in any way with data packs without manual labour (read: in most cases extra commands which won't get used most of the time). If you're working in an environment with a mod loader, use the mod loader instead of MC commands. What you intend to create will likely perform faster that way as well.

P.S. - MultiMC might be a good place to look at regarding accessing version.json files.
P.P.S. - It is possible to support moded blocks/items by creating a mod which provides a command for iteration instead of using loop-unwrapping and then writing extensions for this project on top of that. Make your data packs complain if said commands aren't available and disable themselves.
Another option would be writing the mod which just outputs lists during post-init phase and then recompile data packs using those lists. Or make the mod do that.
That being said, I too dislike the fact that .mcfunction files don't have .lua extension or that some very crucial commands are being overlooked every update. Minetest doesn't have this problem for instance because it was written with extensibility in mind from the beginning.

@chorman0773
Copy link
Contributor

chorman0773 commented Apr 28, 2020 via email

@sancarn
Copy link

sancarn commented Apr 28, 2020 •
edited
Loading

@Caellian I agree in principle.

There may be a way to support mods though. You (or this project) defines a command interface for mod extensibility. Mods/mod frameworks conform to that convention and set some flag to true. E.G.If you declare there needs to be a command like IterateBlockTypes <namespace> which does gives you the blocktypes to iterate over (using some method) then mod framework owners (like Forge) could implement that interface into their frameworks.

P.S. I haven't messed with commands in about 2 years, so am not really clued up with the current systems and whether this would be possible. All I'm saying is typically that's how other software deals with unknowns as far as extensibility is concerned.

P.S.S if we're talking about decompiling jars, again mod frameworks like Forge could provide compile-time APIs as well, if thoses lists are required at compile time rather than at runtime. You can then at least tell users to compile the datapack for the version they want to play on.

Other iterators which might be beneficial are crafting recipes btw

@chorman0773
Copy link
Contributor

@sancarn as it stands, crafting recipes cannot be iterated statically, even in vanilla. Because Recipes are data driven, you cannot assume that any given recipe does or does not exists. Again, in the minecraft domain, you can probably assume that certain recipes will not exist, but you cannot assume that certain recpies will.
I could probably propose "discovery" commands to Minecraft, and to Forge concurrently.
For at least the minecraft level, it would be helpful to have supporting statements. For forge, the best support would be an implementation, though support on the PR from this project generally could help.

The main problem would be using the results of the discovery command in some meanful way. There is presently no way to store certain types of information, like identifiers, and recover them in a future command. If that were overcome, I see no reason a discovery command wouldn't be useful.
If done statically, the game would have to be started enough that a dedicated server is running, and a command could be injected. It could be UB if then used after a registry change. This would have further implications however with recipes, loot tables, structures, and tags (as well as modded data driven content), that the datapack itself is bound to a set of enabled datapacks, or UB occurs at runtime.

@Caellian
Copy link
Author

@Caellian I agree in principle.

There may be a way to support mods though. You (or this project) defines a command interface for mod extensibility. Mods/mod frameworks conform to that convention and set some flag to true. E.G.If you declare there needs to be a command like IterateBlockTypes <namespace> which does gives you the blocktypes to iterate over (using some method) then mod framework owners (like Forge) could implement that interface into their frameworks.

P.S. I haven't messed with commands in about 2 years, so am not really clued up with the current systems and whether this would be possible. All I'm saying is typically that's how other software deals with unknowns as far as extensibility is concerned.

What would be even better is asking fameworks to do their block registration through data files and to agree to do it consistently. That one line of code that it takes to say "this id uses this class to construct block instances, this map color, this sound id, ..." can be easily replaced with JSON files which can be consistently parsed without decompilation and java parsers and handling edge cases for decompiled code and... Only downside I see is a tiny impact od loading time which is negligible compared to 5min of waiting with 100+ mods.
This would allow very quick gathering of all IDs.
I honestly don't see why Mojang still hasn't added ability to define new blocks this way, it would've saved everyone's time and made players very happy.

P.S.S if we're talking about decompiling jars, again mod frameworks like Forge could provide compile-time APIs as well, if thoses lists are required at compile time rather than at runtime. You can then at least tell users to compile the datapack for the version they want to play on.

No need to rely on Forge. Minecraft can be deobfuscated without MCP or loom mappings as Mojang (finally) provides official mappings. It's just that no one wrote "renamer classes" for fernflower yet that consume proguard format.

Other iterators which might be beneficial are crafting recipes btw

Crafting recipes are the reason I suggested ability to load JSON files. New versions store a lot of stuff (mob drops, worldgen chest content, recipes) in those. Mods use them a lot too. For instance, adding block variants in Chisel is done via tags in new versions (also a data json file).

@chorman0773
I wanted to add that nothing apart for positions, entities, ints and item counts can currently be iterated. If you want iteration, data has to be loop-unwrapped and baked it. Best that can be done from my perspective is provide sources of your commands and allow players to regenerate commands if new data packs or mods have been loaded or the game version changed.

Also, listing IDs isn't magic stuff. Every mod loader has registries for all things which are useful to iterate over, they are consistently iterable due to underlying data structures. It doesn't make sense to change them not to be iterable. They are also publicly exposed to all mods which makes this very easy. Registry code belongs to mod loader and changes even less often than MC code for handling blocks. You can keep a mod up to date with minimal effort.

Shop system doesn't have to deal with events. In fact doing it that way is unnecessarily bad for performance. Haven't written mods in a long time but I believe you just have to write a custom AI and reuse MC mob models if you're doing it through entities or write a custom block entity which relies on very stable MC code and is very unlikely to break. MC block entity code (apart from the name) hasn't changed in a very long time nor I can see it changing (at least what you'd use) in foreseeable future.
You only have to emulate and use events if you're doing it with commands.

@chorman0773
Copy link
Contributor

chorman0773 commented Apr 28, 2020 via email

@Caellian
Copy link
Author

It would place a huge burden on the user of a datapack generated using CBA.

No, it would allow them to fix the data pack to work for their specific configuration. The way things currently are it's the best that can be done. I know most of players aren't exactly technologically savvy, but this would allow server administrators to fix the data pack without having to manually edit compiled commands (which I assume aren't very pretty).

They would have to ensure the exact environment the pack is deployed in is identical.

Again, this limitation is imposed by the way data packs work. My suggestion alleviates the pain of long term maintenance (from writing/changing 1000 lines of commads to changing a simple config file and rerunning the compiler.

A change, even in a version of the datapack, could break that pack.

This statement makes no sense to me. Of course, change in functionality will require a recompile. That's how programs work. If you take any C/C++ program and change it's compile time configuration you'll need to recompile to get and use the new binary version.

This would mean that in the presence of this feature, datapacks could never be semantically versioned.

That's not how SemVer works. You're not versioning the environment or compile time configuration, you're versioning your code. DataPack is the end product of compilation and as such it's insignificant for semantic versioning scheme.

I am working on a way to make this work, but as it stands, it would only work in a very specific modded environment.
Once I have it working there, I can propose it to mojang via the feedback site.

Good luck with that. As long as your work is tied to MC code it will break. All code breaks, but MC code by comparison changes much quicker and this means that you'll have yet another quickly changing part of the system to which you'll need to adapt to and take care of.

Regarding feedback, I don't remember the last time Mojang actually listened to peoples suggestions relating to their code. It almost always gets dismissed as "impossible because X relies on Y" instead of them changing Y and making X more flexible (which would in turn make their jobs easier). Only suggestions they listen to are suggestions which are asked for by a lot of players and those usually tend to lean towards "add this block", "change this", "make this do that" instead of actually useful features with potential to generate much more content. It's also easier to write 10 lines of code which add new blocks than to (for instance) revamp how NBT works, store data directly in it and allow /data to modify any game data without recreating the entity.

Expecting them to change something (even if you provide a ready solution) is very optimistic. Don't get your hopes too high, many have done so and nothing changed. That being said, when you do post your suggestion, forward me a link so I can upvote it to make it more relevant. I'd love these changes too. I'm just burnt out on hoping they will improve some critically bad design decisions.

One of my primary goals would be to allow datapacks to be perfectly stable as an environment, both between minecraft versions, and modded environments.

Nothing you make for MC is going to be stable and will require constant maintenance. This is part of the reason I'm no longer writing mods. Apart from being a huge time drain, they're not very useful for CVs and I'm trying to make my life easier.

Publically exposed to mods, not to functions. Have fun iterating registries externally.

Again, unrelated to this issue/suggestion. It's up to Mojang to fix their design choices. Best we can do for now is handle this content statically. If you don't want your data pack to require recompilation on environment changes, don't use proposed features.

Also, things like recipes, advancements, etc. I haven't had much time studying the code. However, they are not stored in registries, at least not the same way blocks/items/etc, are.

These would be accessible through JSON command. mcfunctin design issue, not related to this.

Registries are locked well before any kind of server can start. This has been the case in forge since 1.13, and is by design. I do not believe they are as easily iterable.

As a data structure they should be iterable. Their content is stored in a map. You don't need them to be "unlocked" unless you plan on modifying their contents. I don't know much about how they work, but NEI/TMI/JEI does access a list of blocks and items in order to show them, so they are iterable. Or you can build your own collection using some form of ASM and inserting a function.

Functions in particular I believe are not iterable, the function manager only has lookup methods IIRC. IDK about recipes or loot tables. This may or may not change, and I am not in any particular hurry to pr that to forge.

I don't see a use in iterating over functions. You can use tags for specifying function lists which are meant to be called as interfaces. Iterating over arbitrary functions, especially calling them is bad design and would likely mess up worlds or other data packs.

Also, custom entities/block entities will only work if you also have control over the client, which part of my limitations is that I do not.

Right... another MC design problem. This is doable, a little complicated to write the networking code, but in theory it is possible to make entities purely server sided and just send the client 0.5MB of data required to render them. Again, up to Mojang to stop hardcoding stuff. 70%ish of currently hardcoded data can be made modifiable through configuration files.

Regarding mods and mcfunction limitations: They are not related to these suggestions. If you want we can chat on Discord/mail about these but it's just spam putting it all here in this issue. I bet it would have a much bigger impact if we lead this discussion on MC official Discord than by making this issue less readable and discussing it here.

@Caellian
Copy link
Author

Caellian commented May 7, 2020

@simon816 Added raytracing to the list.

@chorman0773
Copy link
Contributor

chorman0773 commented May 7, 2020 via email

@Caellian
Copy link
Author

Caellian commented May 8, 2020 •
edited
Loading

I was trying to say that changes in other datapacks break cbl datapacks. I'd argue not all data packs are generated, and in practice, the breaking features usually are not, except when the datapacks are part of a mod. I primarily use forge datagens for loot/recipes/advancements, even for standalone packs, but as far as I am aware, few others do so as well. Also, your point about changing the configuration, yes you have to recompile the program, but if its a library, you don't necessarily have to recompile everything that depends on it (provided the feature sets are ABI compatible), and you certainly don't have to recompile literally everything on your computer when you flip a configure flag on some library (exception being of course something like glibc on linux, where just about everything depends on glibc).

"(provided the feature sets are ABI compatible)" is the crucial part here. I'm not sure how to be any more explicit in saying that changing blocks handled by your data pack will most definitely be a breaking change - with or without this feature. There is no way around it currently. If any other data packs depend on yours, they will have to recompile/rewrite parts of their logic to accommodate changes in yours.

Changing the list of available blocks is like disabling/enabling exceptions or certain functionality in a library. It wouldn't be if there was a way of storing IDs and using them elsewhere, but as there's no such option, it is. As such it is actually an "ABI" change and cannot be handled any differently.

I suppose you have never heard of an Application Binary Interface or stable versioning of that. Shared Libraries are versioned because certain changes break applications depending on them. The feature you propose means that all changes break the ABI of a datapack. This is my problem with that feature.

Ad hominem. I'd argue that there is no ABI when it comes to dynamically interpreted languages or in case of CBA a VM because neither of them is actually a binary format. CBA is not actually a compiler, it's a transpiler adding a custom set of features (a compiled-in dependency) to allow a bit more flexibility when it comes to data mangling during command interpretation. So, we're talking about API versions here.

Data packs haven't been designed to work together or depend on each-other as mods do. I feel you have too high expectations from a very limited feature set being actually provided. While commands are in theory Turing complete, integration with the actual game state is very limited in incomplete.

My problem is that it would become impossible for me, as both a modder and datapack developer, to expose a stable public api for datapacks in the presence of this feature, not that my datapacks in particular are broken by an opt-in feature that breaks between environments.

If your API doesn't use said feature it wouldn't prevent you from exposing a stable API. Again, there is no better way of doing this. Another important thing to note is that majority of data packs are written for the vanilla game and support mods by accident.

Arguably, design limitations inhibiting the feature are very relevant. If you look to, say, the C++ standards committee, one burden paper authors have to bear is showing that it could be feasibly implemented, usually by providing implementation experience.

It's not unimplementable. It is very possible and if you go through the entire issue you can see what is required to implement it and even make the iterated data self-maintainable.

I am offering feedback based on my use case to your suggestion. I'm sorry if I was mistaken that you were asking for comments on the features.

You keep bringing up how the feature wouldn't allow data packs to operate together and work in modded environments but offer no alternative solutions. Yes, I'm aware and I pointed out several times that this wouldn't allow mcfunctions to be reused by other data packs and that they could break on content changes. That's not the point though. There is no way of defining a robust system to handle this currently. You keep on insisting that this feature shouldn't be implemented because things would break, but they break anyway with or without it. The only alternative is to force data packs to write their own iterators and collect data they need on their own which is much more error-prone than what I'm suggesting. Even if you use something like Item("thaumcraft:wand") it's up to the data pack author to accommodate any new content when new features get added. Eventually, every data pack definitely requires maintenance, no matter how it's written.

This would only allow people to somewhat reliably iterate over vanilla content without having to collect all of that information on their own. Data aggregation can also be automated to a certain degree (as I've mentioned) which is far better than someone hard-coding list of blocks for instance.

So, instead of saying that the functionality shouldn't be implemented because it doesn't allow for some functionality you'd like, say what you'd like to be differently done so that it does. So far you've been ranting on and on about supporting mods, but I see no better alternative. It's a small utility feature, not a be-all-end-all solution to fixing bad vanilla command design choices or missing crucial functionality to make mcfunction files actually a viable option for scripting content. There is only so much that can be done to make peoples lives easier and you keep fighting it as if what I suggested is in some way harmful when in fact it makes things a lot easier.

simon816 added a commit that referenced this issue Jan 10, 2021
@simon816
Initial work on a Container API. See #16
bf4918b
@simon816
Copy link
Owner

I've been making a bit of progress on a Container API.

I've added Container.get_item_count

The following code snippet gets the number of stone blocks within a chest at position 193, 56, -11 (where I created a chest in a test world)

include "Items"
include "Container"
include "Text"

void main() {
    Container c(_IRType("block_pos", 193, 56, -11));
    int num_stone = c.get_item_count(Items.stone);
    Text t;
    t.append_ref(num_stone);
    t.send_to_all();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@simon816 @Caellian @chorman0773 @sancarn