Blossoming Pots

[runcows]

Blossoming Pots initial commit.
Ready for code review and further discussion, I think...

[misode]

I would probably want to limit the number of display entities per pot to something like 5. Anything above that I feel goes beyond of what "feels vanilla". Especially the coral decorations, where it's using 41 display entities for a single pot. That's not only a performance issue, but I also don't think it looks good visually, since there are too many small details and you just don't see that with vanilla. This already has the coral fans, I think those are a better fit.

I was a bit unsure on how this module worked tech wise, with the storage. From my point of view coming from beet, I would probably generate a mcfunction for each supported item, which just had the item display summon commands, that would probably allow you more flexibility without confirming to a strict storage schema.
I think it is probably fine to keep using the storage, unless you come across a major hurdle.

I didn't do a full technical review yet, but it seems fairly well put together after a first look.

[runcows]

I would probably want to limit the number of display entities per pot to something like 5. Anything above that I feel goes beyond of what "feels vanilla". Especially the coral decorations, where it's using 41 display entities for a single pot. That's not only a performance issue, but I also don't think it looks good visually, since there are too many small details and you just don't see that with vanilla. This already has the coral fans, I think those are a better fit.

Totally get it. I feel a little bad removing the coral blocks because I worked hard on them when I made them, but I totally get it and am down to axe them. I'm curious on your thoughts for a limit of display entities. There's only 5 other things that have more than 5 displays. The next highest count after Coral Blocks is the 3rd stage of Chorus Fruit with 18, which im much more hesitant to remove. After that is Kelp and Twisting Vines which go up to 16 and I'm willing to chop those down a good bit. Bamboo and Glow Berries have 8 and Weeping Vines has 9, again willing to chop those down a little bit.

[Bloo-dev]

This is not a full review (yet), but I had a couple of minutes spare. More to come later.

[runcows]

Pushed more changes. I think this covered everything we discussed in the review session :D

[TheThanathor]

Code looks great! I've left a few comments, but it seems good to go.

I've also tested it in game and found only one very minor bug:
When a custom plant is in a flower pot you can plant a vanilla flower in there by holding right click and walking in circles around the flower pot (the module tries its best to prevent you but you can get a flower to slip through). This does fix itself by dropping the custom flower though so I don't think it's a problem.

[TheThanathor]

Do we know if using macro's here is better than setting the items data from the entity?

[runcows]

I have no idea. My thought a long time ago was that it was better to summon the item with the data than to modify it after summoning.
I think we would have to use 2 selectors to set the item data from the marker; once for the id, and again for the item count. Personally, I just don't know the performance comparison between 2 selectors and a macro

No time to review