[Question] MapDisplay API: Assign MapDisplay to existing map items in a world + tiling question

[enveeed]

Heya,

I have another question about the MapDisplay API. As minecraft identifies their map items with these short map IDs, BKCommonLib seems to try and find an unused one for every new map item created. The problem with this is, that it obviously doesn't support it when other plugins use these IDs to send their own image data to the clients and thus it tends to have both images mixed up. I currently have such a plugin installed on my server and I would like to write a plugin with the same functionality but using BKCommonLibs MapDisplay API as I think its way more handy and it would go well with my other plugins using the MapDisplay API.

Now the situation is, that I have hundreds of map items already in item frames everywhere in my world and I want them to keep their images. To achieve this, I will read the files of the old plugin which includes the map ID and the image data and use it for my new plugin.

I have thought about looping through all map items in item frames in my world and add the fitting NBT tags to them so that BKCommonLib can handle them as normal MapDisplays. Is this the correct (or only) way to do it or is there a way I can do it simpler?

Also, could you provide a small explanation on how to use the brand new tiling system you implemented? Because that would help a lot with this too. :)

Thanks in advance!

[bergerkiller]

that it obviously doesn't support it when other plugins use these IDs to send their own image data to the clients and thus it tends to have both images mixed up

Your existing item frames will continue working. BKCommonLib will not assign a dynamic map id that is already being used. To so, use the dynamic map items created using createMapItem (see down below). You absolutely should not be looping map Ids, its precisely why I implemented this system. I ran into annoying Id clashes when both Traincarts and MapLands needed map Ids, pretty much requiring me to come up with a better system. That better system is based on UUID's that are automatically assigned a map Id on demand.

Long answer:

It avoids most problems by 'exempting' maps found on the server (in player inventories or on item frames). It tracks these ids and marks them as "DONT USE THESE!". If a dynamically allocated map id ends up detected as static, the map is assigned a new Id automatically in order for the legacy static id to work. The map might shortly show the wrong map, but correct really quickly after.

Basically, it makes sure that legacy ids stay the same, and assigns dynamic ids only when they aren't already being used. It's fairly complex stuff, but it has it.

Static Ids are forced here: https://github.com/bergerhealer/BKCommonLib/blob/master/src/main/java/com/bergerkiller/bukkit/common/internal/CommonMapController.java#L386

Tiling is very simple. You just put the same map item in adjacent item frames. BKCommonLib automatically detects this and assigns different dynamic id per tile. It does so by basically virtually altering the item stacks themselves before they are received by the clients. The map display interface sees only a single 'image', not the size of 128x128, but whatever dimensions are needed to display it on the largest tiled display found. With getWidth/getHeight() this info is obtained. Displaying a multi-tile display is no different from a single-tile display.

Difference between dynamic and static: dynamic map ITEMS store an additional randomly generated UUID code in the item's metadata. BKCommonLib sees these and automatically handles all the id generation. The item's durability stays 0.

With these items whats even more awesome, is that your map display is automatically created once somebody views the map item. It does this by storing the class and plugin owner of the class in the item. This makes the API very simple to use: https://github.com/bergerhealer/BKCommonLib/blob/master/src/main/java/com/bergerkiller/bukkit/common/map/MapDisplay.java#L1058

=================================

To make your own tiled display, you can set the item in all item frames, or give the player the map item and let him fill the item frames himself. (this is easiest in creative mode, admittedly)

Maplands is probably the best pilot example of all this. https://github.com/bergerhealer/Maplands/blob/master/src/main/java/com/bergerkiller/bukkit/maplands/Maplands.java#L66

Simplest example:

public static class MyDisplayClass extends MapDisplay {

    @Override
    public void onAttached() {
        getLayer().fill(MapColorPalette.COLOR_RED);
    }
}

ItemStack item = MapDisplay.createMapItem(MyDisplayClass.class);
player.getInventory().addItem(item);

If you want the same map elsewhere, clone the item and put it in another item frame or someone's inventory. There's a lot more cleverness hidden in the API, such as that it will automatically synchronize the map item at all places it is displayed when you change it from within the Map display. This effectively allows you to store settings in the map item itself.

I really should make a proper wiki page for this, but I'm also spending a lot of time programming :)

[enveeed]

@bergerkiller

Thanks for this detailed explanation, it really helped me to understand how the lib handles the map IDs. Also, sorry for abandoning this issue for so long, I just started working on my project again. :)

While understanding and using what you told me, I came across some new issues while writing my plugin. To help you understand me, here is a small explanation of what I am trying to achieve:

Basically, the plugins function is displaying images in Item Frames, in any size and any amount. Every image-map shall be one MapDisplay instance everywhere the image is used. So a place with one or multiple map items all have the same MapDisplay instance, but at another place the maps should show the same image but be a different MapDisplay instance (to make it possible to have different sizes). The tiling ability is used to stretch the image over multiple Item Frames. This is intended for a creative mode map / server, so the amount of maps available isn't of matter.

---

1. Is it possible to pass default MapDisplayProperties to a MapDisplay when created?

I noticed that you added the properties field to MapDisplay since we last talked about it, which I am very grateful for, since it makes state persistence a lot easier. However, is there a way I can pass default MapDisplayProperties to the new MapDisplay created when viewing the map item created via MapDisplay::createMapItem ?

Let's say I have an image with ID image1 and want to give the user a map item displaying this image. I have to make the MapDisplay instance somehow know which image ID it has to show. If it already knows the ID, I can safely read and write it from and to properties, but when the map item is new I have no way to put the ID image1 in the properties field, so I have to go via the old NBT tag method.

Is there currently a way to do this? If not I would love to see something like that in the future, as the NBT method is rather annoying. ;)

---

2. Tiling sometimes not correctly updating tile maps

For the auto-adjustable size I use this piece of code in my MapDisplay::onTick implementation:

if(sizex!=getWidth() || sizey!=getHeight()) {

    sizex = getWidth();
    sizey = getHeight();

    updateImage();
}

The sizex and sizey are fields to cache the current resolution and detect if the resolution changes. ::updateImage is resizing and drawing the image (in a separate thread to not slow down the main thread)

This looks like a pretty failsafe way to detect changes in the size, but the problem with this is that it does not detect changes in the actually available tile maps. This means that if the full size is already set but maps are added in an area where the full size doesn't change because of it, they somehow never receive the image data, which leads to funny behavior. I think BKCommonLib should cache the data when its drawn and then automatically update maps added to the full image. Either this is not the case or it doesn't work reliably in my scenario.

Also, here is the full VTestMapDisplay.java I used for this scenario.

You can see the problem I am talking about in the image above, the 2 empty maps should display their tile data but don't. I already received the Updated image 384x384 message at this point, so it should work.

---

3. Tiling sometimes shows wrong data on single tile maps

In the image above it is visible that the tile at the bottom right doesn't show the correct image data, but rather another part of the image (The item frame is not rotated). I can't for sure say in what cases this happens but it basically happens very often when putting maps in the wall of item frames. Not all the time though. When this happens, the Updated image 256x256 was already received so drawing was already done.

I tested this and 2. without using an extra thread for the updating logic, but it still happens. The only difference is that it makes the whole server way slower since it does the scaling on the main thread.

---

4. Tiling slows down the main thread

I must say, other than this, I am amazed at how fast this API reacts in all other ways, great work! :)

I am not sure if this can be optimized with the current infrastructure, but I noticed that tiling is incredibly slow. Whenever I place a new map in a frame on a wall full of other frames, it temporarily halts the server noticeably. I suppose this is due to the logic being done on the main thread. I don't know if it's possible to calculate all necessary information in an extra thread without breaking anything, but it may be a possibility to make the whole thing a bit faster and more usable.

---

Thanks for taking the time to read through all this, and for providing such a great API! I was looking a long time for something similar and it's awesome how great this works and how easy it is to use! :)

I am fairly sure that these problems aren't on my side, but if they actually are, sorry in advance!

[bergerkiller]

1.) You can add additional NBT metadata to the item created by createMapItem. The properties field is simply a proxy to the NBT data of the item. BKCommonLib has the 'CommonTagCompound' api for it:

ItemStack mapItem = MapDisplay.createMapItem(MyDisplay.class);
CommonTagCompound tag = ItemUtil.getMetaTag(mapItem);
tag.put("key", "value");

2/3.) I haven't tested it in a while though, if its glitchy then it needs to be looked at. Swapping out the tiles is a bit finnicky. I'll have a look at some point, you can consider it a bug.

4.) Yeah, discovering the tiles is a bit complicated. It involves a lot of iterations around all entities to algorithmically spread and detect other tiles. The larger the display, the more tiles update themselves. There is room for improvement. I consider keeping track of a map of Block to Item Frame entities so that there is no more need for iterating entities. The logic is a bit complex, but it could be worse. By looking at columns and rows in one go the complexity is reduced. https://github.com/bergerhealer/BKCommonLib/blob/master/src/main/java/com/bergerkiller/bukkit/common/internal/CommonMapController.java#L1442

[bergerkiller]

I tested on the latest development build and it seems to work fine. Not sure where you are seeing bugs, unless youre using an older build. I vaguely remember fixing a few things related to tiling.

[bergerkiller]

Oh wait, after a restart it does seem to glitch out until I re-place the item frames. Odd. I'll try to fix that.

[bergerkiller]

I made some fixes, you can see if it helps to improve your situation. I couldn't really reproduce any further problems with tiles misaligning, if you manage to reproduce it, maybe you can send a plugin .jar I can test with?

https://ci.mg-dev.eu/job/BKCommonLib/336/

[enveeed]

Thankyou, I tried it with the new development and it seems to have fixed 3.. :) However, 2. was still happening.

I used your new test MapDisplay and saw that it in fact worked fine, so the issue with 2. must have been on my side. I noticed that you were drawing the image in ::onAttached which meant that this method apparently also was called when the size changed. My implementation had some heavy loading logic in there because I assumed it will only be called once. At this point I decided to rewrite my whole implementation again and it worked perfectly fine. I am not sure why the heavy loading in ::onAttached caused some frames to stay empty, but it apparently was the cause.

So, except for 2., it seems like my own stupidity and incomplete knowledge of the API lead to these problems.

I really appreciate that you took time to help me, even though it was my fault all the time. :)

This is how it looks now, works great:

---

For 1. I was already doing it that way, I was just looking for a more pretty way to do it without directly dealing with the item NBT. If it is intended to work like that, that's fine, and it works great most of the time. I noticed though, that when I move an Item around in my player inventory, it sometimes happens that it "forgets" my NBT tag. MapDisplay functionality is still working so for some reason it only applies to my custom NBT data.

I will close this issue now, it helped solve my initial problems and answer my questions. But just as a follow up, do you have any idea what could cause this loss of NBT data?

[bergerkiller]

Yeah, you definitely should do the drawing in onAttached. There is also onDetached. When the size changes, it first detaches, rescales, then re-attaches. It doesn't create a new instance. Were you doing the drawing in the constructor before? Or something else entirely?

As for the loss of NBT, the only thing I remember is that in creative mode there is a bunch of fiddling going on. BKCommonLib does a bunch of sneaky trickery to prevent sending the metadata of the item to the clients. This is to prevent megabytes of metadata being sent, since I assume people may store something like image data in there. Creative clients however set slots instantly, as in, they send items over the network. BKCommonLib handles this and fills in missing metadata in the newly placed item. I assume this logic breaks things rarely or sometimes?

https://github.com/bergerhealer/BKCommonLib/blob/master/src/main/java/com/bergerkiller/bukkit/common/internal/CommonMapController.java#L573

A quick way to verify this is to see if it also happens in survival mode. I assume it doesnt.

One potential for breakage I see is when multiple of the same map UUID exist in the inventory. Is this the case? I may have to fix this, though I'm not sure how to best go about finding the original item from creative set slot.

[bergerkiller]

@Enveed You should see quite a significant performance improvement for the tiling refresh logic. It caches the item frame entity list in an intelligent way to minimize useless loops. This also allows the same list of item frames to be used for all iterations of findNeighbours.

https://ci.mg-dev.eu/job/BKCommonLib/341/