-
Notifications
You must be signed in to change notification settings - Fork 187
World edit commands for Cubyz #1141
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.
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
World edit commands for Cubyz #1141
Conversation
|
Oh and also this PR adds schematic storage format as part of |
|
If possible, can you add a wand item? It doesn't need a fancy texture |
Sure, but in separate PR - we are already near 500 lines - I think that I should also leave rest of the features listed (maybe except schematic loading) for separate PR. @IntegratedQuantum won't be happy about reviewing my spagetti 🥲 |
|
At a glance I already see a few issues:
|
I agree, but this is a quick way to do it and I want to support a human readable format for sake of being able to sit and scribble something in notepad if needed. As you already said, Cubyz is not ready for large amounts of block updates, so let's maybe avoid premature optimalization for sake of having features, what do you think? Regardless of what I just said, I agree that this should not be a primary storage format, I can and will work on a binary format, but I don't want to do it in this PR, IMO this is beyond providing simple WorldEdit functionality, so it is beyond of my goals right now. On top of that I would like to also at least partially support imports from minecraft schematic format to a reasonable extent with some limited translation table, but that is a material for yet another PR.
I think they are doing that, the palette stores only block type, but block array stores both, unless there is a bug in the code.
You have to translate from block palette that was present in world you exported from to block palette of world you are importing into, at least because they could have been created on different versions of the game, so there must be a palette storage. Since most of the time you will not be using whole palette, there is also no point in storing whole palette, which can have up to 65k entries. Therefore palette is created with blocks that are necessary and reassigning IDs ensures there are no gaps in ID list. For rotations and flips you will have to translate it back to world palette regardless of what palette you use for storage, as rotations and flips must also work on schematics you load from drive.
This is what WorldEdit does, sure, we can do something different but then you have to transfer schematic to client and that is more complicated for limited benefit. Users of those commands are expected to have admin rights, thus also access to the server.
That's great, I didn't know that!
I guess this is it for sticking with WorldEdit design. I know, It's Cubyz, not minecraft, but being unique is not always an advantage, especially from user experience it is usually better to follow standards that were established before. Anyway, that extra slash shouldn't be a big deal so I will get rid of it, I just had to talk back 😝 |
OneAvargeCoder193
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are a couple issues with the way that you store the selected blocks
|
@OneAvargeCoder193 I moved pos1, pos2 and clipboard to user data. |
The quick way it is going to screw us over in the long run. And it isn't even that much quicker than the alternative, instead of doing zon.get, you just read stuff at an offset. And Also Cubyz may not be ready for large block updates, but if it's there, then I'm sure that @OneAvargeCoder193 will be tempted to use it for structure generation.
That should be a third-party conversion tool.
If it does that, then please use the
Ah, if it's only for storing the file, then it should only be used once when loading it to translate the block ids. Doing this translation every time a structure is pasted, is just wasteful.
I was thinking, maybe a player would want to take a structure they built in another world and paste it one some multiplayer server or something like that. But it's fine with me if you don't want to support that. |
|
There is literally 0 error handling while loading tho, so there is still a bit to be done before making it ready for review |
|
Before you put any more work into it, I'd like to do some improvements to how other binary formats are handled, to improve error handling, among other things. |
How is that a bottle neck for this PR? |
Well you did say you want to implement more error handling, this is me telling that you should wait. |
|
Well, I ment to add some logging / return errors instead of panicking on everything, I did not anticipate it would stall this PR. |
|
Well it won't stall the PR, but since this is not going to finish review in the next weeks, you might as well wait with it, until I implement better options. |
Good to know |
IntegratedQuantum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have tested this and found one small issue with blocks that require support: #1217
However, please do not add a fix to this PR, since it is non-trivial to fix and I would like to get this merged since it's blocking structures.
Co-authored-by: IntegratedQuantum <43880493+IntegratedQuantum@users.noreply.github.com>
Co-authored-by: IntegratedQuantum <43880493+IntegratedQuantum@users.noreply.github.com>
Co-authored-by: IntegratedQuantum <43880493+IntegratedQuantum@users.noreply.github.com>
| } | ||
| source.worldEditData.clipboard = Blueprint.load(main.globalAllocator, storedBlueprint) catch |err| { | ||
| std.log.warn("Failed to load blueprint file '{s}' ({s})", .{fileName, @errorName(err)}); | ||
| source.sendMessage("#ff0000Failed to load blueprint file '{s}' ({s})", .{fileName, @errorName(err)}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you do this everywhere now, how about a helper function to avoid duplicating the messages everywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not resolved
Co-authored-by: IntegratedQuantum <43880493+IntegratedQuantum@users.noreply.github.com>
d6c87ef to
11d8e4d
Compare
|
Btw extracting blueprint file io to struct made the code longer by about 20 lines 😝 |
Why did you do it then? |
This reverts commit f0bbe50.
IntegratedQuantum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you did to the blueprint command really was that helpful, especially if you did increase the number of lines.
Cause I hoped it will be a good abstraction. I have reverted it tho, and moved just the opening into separate function, it is still +1 line of diff. |
|
How about just a helper function for the two output calls: |
|
Done. |
|
Did this finally save some lines instead of adding them? |
Co-authored-by: IntegratedQuantum <43880493+IntegratedQuantum@users.noreply.github.com>
Yes, total of 4. |
IntegratedQuantum
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I think we are done here.
Honestly if it weren't needed for the structure models, I don't know if I would have had the motivation to keep reviewing this. In the future, please please please try to make smaller PRs. Reviewing 650 lines just takes so much time and energy, and I'm sure that's true on your side too.

This pull request introduces some of the commands available in very popular minecraft mod called WorldEdit.
List of WorldEdit commands can be found here
For best user experience I decided to take approach of implementing those commands as close to original as possible.
CHANGELOG:
/pos1/pos2/copy/paste/blueprint save/blueprint load/blueprint delete/blueprint listEven if Cubyz engine is not ready for large amounts of block updates I think that players don't really care and will be happy to have those features despite potential lag spikes.
I do not plan adding brushes in this PR, it will be waaay too much code at once. For that reason
rotateandflipare also considered optional, maybe they can be added in a basic version where they just move blocks around but don't update block data to eg. flip stairs. Pattern expressions are also outside of the scope of this PR, for now commands that take patterns will be only able to take one string block ID.Related to: #1214