mrkite / minutor

Mapping for Minecraft
http://seancode.com/minutor
BSD 2-Clause "Simplified" License
278 stars 52 forks source link

Added ChunkLock visualisation. #369

Closed madmaxoft closed 9 months ago

madmaxoft commented 10 months ago

This adds a new View option, "ChunkLock", which frames in red all chunks that are currently locked by the ChunkLock resourcepack: image

Also adds a note in the status bar, displaying the item needed for unlocking.

madmaxoft commented 10 months ago

For the impatient, here's a Windows-compiled version (Sorry that it needs a ton of supporting DLLs, I don't have a static Qt available) https://xoft.cz/Minutor_with_ChunkLock.7z

EtlamGit commented 10 months ago

As I'm far away from any computer, I can not look into the code any time soon. Therefore, I start just asking some questions:

It seems to be a datapack and not Vanilla feature. Therefore, I would expect the new menu entry only to show up when this datapack is detected. This might be something for WorldInfo class during initial parsing of the world folder.

The current highlight seems to be a red border around the Chunk. With this, the border pixels are covered and not visible. Have you tested other rendering options? E.g. red shadow of complete Chunk.

Can you think of any way to describe this feature as plugin in a JSON file and have some generic code supporting more mods of this kind? I mean a JSON file to describe how to detect that mod, how to detect mod data per Chunk, and a colour to render that Chunk. We could try something like this as second step. I do not mean you should do that now, just think about it.

madmaxoft commented 10 months ago

I haven't looked into hiding this the menu entry when the datapack is not present. Since the entry is effectively a no-op in such a case, I don't think it's super necessary to do so.

As for the highlight, it's always 1-pixel wide, even with zoom, and the assumption is that one uses pretty high zoom levels when using this datapack anyway, so it should be good enough. With overlays over the whole chunk, we lose a lot of the information - suddenly blocks are different colors. I'm open to suggestions here, this is just something that I found works best for me right now. I could quickly change the rendering and provide an image for comparison.

I've tried coming up with a way to describe such a feature in a generic way, but it's way too complicated - the datapack uses specific entities within each chunk, with specific tags, to store the needed information; since Minutor doesn't even store the complete entity info, it would mean a huge change in the data representation (and possibly performance). For rendering purposes, it makes much more sense to actually mark chunks with the properties, rather than keep an extra list of chunks to highlight. Not to mention this PR still doesn't provide the entire information, it only shows what kind of item is needed for unlocking, but doesn't show the amount needed - that is stored in the scoreboard, which aren't parsed by Minutor at all. In short, if there was a system to represent all this in a sufficiently generic way, it would be too specific for this datapack anyway.

Oh, I do see a way to do it generically: Add a Lua interpreter and some API, then we can make Lua code snippet to detect and parse and set the needed information :D Bring out the big cannons.

madmaxoft commented 10 months ago

Here's a comparison of the rendering between shades and frames: image image image image

TizzySaurus commented 10 months ago

Just wanted to say this is super cool and can't wait for it to get merged!

Fwiw I definitely prefer the shaded representation of locked chunks (covering the entire chunk rather than just outlining), but not sure how well that would play with other visualisations active (such as spawners/igloos/etc.).

madmaxoft commented 10 months ago

I've been trying both visualisations for some time and I have to say that although the full-shading looks better at first glance, the framing actually feels much better for actual use. But since two people already said they'd prefer the shading, I'm willing to change my PR.

EtlamGit commented 10 months ago

(I just asked if you compared them, I have no strong opinion yet. Probably I also have to try it out.)

@madmaxoft @TizzySaurus could you please provide world downloads for testing?

madmaxoft commented 10 months ago

@EtlamGit Here you go, this is the world used for the screenshots above: http://xoft.cz/minutor/ChunkLock.7z (116 MiB!)

madmaxoft commented 10 months ago

@TizzySaurus Look at the single locked chunk at the center bottom in my comparison screenshots, just above the kno in Unknown Block in the status bar. With shading it's very hard to see that it's actually locked, it just vanishes with all the dirt around it.

TizzySaurus commented 10 months ago

@madmaxoft

@TizzySaurus Look at the single locked chunk at the center bottom in my comparison screenshots, just above the kno in Unknown Block in the status bar. With shading it's very hard to see that it's actually locked, it just vanishes with all the dirt around it.

I personally don't have any issues recognising it, but maybe that's just a difference in monitor/lighting 🤷

I've been trying both visualisations for some time and I have to say that although the full-shading looks better at first glance, the framing actually feels much better for actual use.

I don't feel particularly strongly either way, and I can see cases where the framing would be more practical, and since that's what you've already got here I'm happy for it to stay the same. Have been doing some messing around with the other overlays and it seems the current framing is clear even with those on, so yeah, maybe best to just leave it.

@EtlamGit

could you please provide world downloads for testing?

I've got one here: https://www.mediafire.com/file/zrjfdxfue5tarvj/world.zip/file

EtlamGit commented 10 months ago

(I'm starting to test now)

I still think the menu entry should be hidden until this mod is detected. It is a non-vanilla feature and only a minority of users even know what this view mode is about. So it should not be visible until it makes sense. For Overlays it is the same, only Overlays that are present in the game are shown to the user, others are not confusing him/her. The current view modes are always valid and contain information that is possible to render. To make that even more clear, there should be a separator in the menu to divide vanilla view modes that are available for all users and view modes that are depending on installed mods: grafik This separator should of course only be added when a modpack view mode is available.

Detecting this modpack is easy: in level.dat there is information when it is active: grafik

From my first impression the current rectangle around the Chunk might be fine. This would emphasize the difference to current view modes.

madmaxoft commented 10 months ago

I still don't think the datapack detection is robust enough - it is based only on a filename, so if someone downloads the datapack under a different name (say the devs release the pack with a version in its name, or someone not english-speaking renames the file) the detection would get broken. It is also possible to have this datapack installed but deactivated, or the opposite - not installed, but still active (somewhat broken).

This probably means the only reasonable way to detect the datapack is actually encountering the specific entities in the chunks themselves. But that is very late.

EtlamGit commented 9 months ago

When the pack is not enabled or the installation is broken, it is task for the user to fix it before he can play again. Therefore we do not have to support broken state. But you are right, we have to support renamed datapacks. And even more, the second example world used an unzipped variation of that datapack (yours is zipped).

371 adds reliable check for available datapacks. The name is taken from JSON inside of each datapack.

EtlamGit commented 9 months ago

Here is a working dynamic menu for ChunkLock: https://github.com/EtlamGit/minutor/commit/b7fbd5d172f5bd67dbe32af9becd2dea582b50d1

It is only the menu entry, toggle flags and signal->slot connections are already in your code. It has just to be combined. I would also suggest to use a new toggle flag to separate vanilla and datapack related view toggles.

madmaxoft commented 9 months ago

I've decided against dynamic action creation and instead opted to have the action always present and only hide it if not detected. Qt already hides separators if two or more are next to each other, so the solution becomes trivial.

I've also modified the drawing code a bit, added an adjustment for when the drawn chunk crosses the X or Y axis (in viewspace) - the rectangles were missing their bottom or right edges otherwise, most likely due to rounding / overdraw errors.

Before: image

After: image

EtlamGit commented 9 months ago

I also observed the behavior of hiding the second separator. But I just got confused by this. Did you find an official description for this?

I tested "fixed UI and then hiding" also, this is obviously easier. Then tried to get the other variant running, as I still have in mind to have these DataPack dependent additions as generic config files. But for now this is not necessary, it is only one DataPack at the moment.