1.13 - The Technical and Aquatic Update #1704

Cybermaxke · 2017-11-27T11:59:43Z

Very WIP, I am opening this PR now to start a discussion on what can be reused/needs to be removed. Also designing a system to easily get variants of certain block types. For example getting a slab with the material oak.

Related to:
#1715 - The variant system
LanternPowered/Lantern#48
#1742

We have to decide what to do with the following data types, could it be reused for the variant system or should be deleted. I already removed a few types, feel free to comment if you don't agree with me removing them.

Modify/remove:

Catalog types and data:

Other data (and keys):

SeamlessData - Removed
SpawnableData

Tile entities:

Note - Removed
Skull - Removed
FlowerPot- Removed

New/Modified

Data (probably separate PRs):

LitData - New (furnaces, redstone torch, redstone)
InstrumentData - New (note block)
SlabPortionData (and SlabPortion) - New
ChestAttachmentData (and ChestAttachmentType) - New
InvertibleData - New (daylight sensor)
SurfaceAttachmentData (floor, ceiling, wall) - New (buttons, lever), is this a good name?
BigMushroomPoresData - New (replaces BigMushroomData)
UnstableData - New (TNT)

Tile entities:

Player head - New

Populators:

Merged Shrub and Flower into Plant

Other:

Update statistics, moved to Minecraft 1.13 - Refactor statistics #1742
Update block traits
Update particles
RecordType -> MusicDisc - In 1.13, the name 'record' is no longer used in any translation keys, item ids, etc. (item names were already Music Discs)
~~Remove extended block states? Everything is now controlled server side.~~ - Forge is still going to use them.

VapidLinus · 2017-11-27T12:06:47Z

Just leaving a quick note that this PR seems to relate to my issue regarding how to get a coloured ShulkerBox from a DyeColor: #1667

edit: music disc type, maybe?

me4502 · 2017-11-27T12:25:30Z

I’m of the opinion it’s still nice to have groupings of obvious variants, such as wool colours. Idk how others feel tho

ryantheleach · 2017-11-27T12:52:27Z

Before you get too far into this, remember that the Data API is an abstraction over the base game.

Having this data available, even if not directly represented in the Mojang block properties was entirely the point of abstracting it in the first place.

BlockTrait API on the other hand, was supposed to be a closer to metal abstraction, that was implementation dependent.

Cybermaxke · 2017-11-27T13:51:08Z

@ryantheleach That's why I am opening a discussion on this. And lot (block) based data is still tied quite closely to the MC internals, making it not possible to update without breaking changes (or ugly workarounds). Since there are already a lot BlockType changes, I don't see any issues with breaking the data types as well.

Cybermaxke · 2017-11-27T15:26:09Z

@VapidLinus There is a RecordType catalog type.

gabizou

Some good ideas, but I think I've found a direction I'd like to take this towards.

Overall, I'm going to still have a think about Variety and Varieties to define what you're calling Material. Effectively what it should solve for is defining a Wool Carpet Block as having Varieties.WOOL, Varieties.CARPET, and Varieties.BLUE to describe the block. This way you can determine already that it is a wool block, it's a carpet (so you can gather all blocks that are matching Varieties.CARPET and WOOL, or simply attempt to match all blocks/items with WOOL varieties.

It would be interesting to try and resolve the awkwardness of showing DyeColor as a value instead of a Property of sorts. One way this is retrievable should be through the existing Property API system in place with Data API. Since it is not an immutable data (can't have mutable properties), you can retrieve the whole collection of Varieties in this fashion.

Would love some input from @SpongePowered/developers .

gabizou · 2017-12-07T07:00:20Z

src/main/java/org/spongepowered/api/data/type/Material.java

- * applicable to {@link BlockTypes#QUARTZ_BLOCK}.
- */
-public interface ImmutableQuartzData extends ImmutableVariantData<QuartzType, ImmutableQuartzData, QuartzData> {
+public interface Material extends CatalogType {


After thinking about it more today, I don't like the idea of naming it Material. Maybe call it Variety and then Varieties.OAK and Varieties.QUARTZ could then work. This way you can also have Varieties.STAIR or PLANK even. This way the "type" is not defined as a Material (since stair is not a material, but a variety of a block). This also allows for us to make a combination of varieties for a specific object to "describe" it.

Thinking on it more, Variety could even be introduced in API 7 (given some time this week maybe to solidify it as a possibility), in which a "moving" towards it is somewhat available before the drastic change of some of this data removal.

Thoughts @Zidane ?

gabizou · 2017-12-07T07:01:05Z

src/main/java/org/spongepowered/api/effect/sound/SoundTypes.java

@@ -207,6 +207,8 @@

    public static final SoundType BLOCK_PORTAL_TRIGGER = DummyObjectProvider.createFor(SoundType.class, "BLOCK_PORTAL_TRIGGER");

+    public static final SoundType BLOCK_PUMPKIN_CARVE = DummyObjectProvider.createFor(SoundType.class, "BLOCK_PUMPKIN_CARVE");


Carve or Carved?

Carve. This is the sound that plays when an uncarved pumpkin turns into a carved pumpkin.

gabizou · 2017-12-07T07:02:08Z

src/main/java/org/spongepowered/api/variant/VariantQueryType.java


-@CatalogedBy(LogAxes.class)
-public interface LogAxis extends CatalogType, Cycleable<LogAxis> {
+public interface VariantQueryType<T> extends CatalogType {


What's the point of this? It's feeling very much either like a Property or a Key in some aspects.

gabizou · 2017-12-07T07:02:56Z

src/main/java/org/spongepowered/api/variant/VariantQueryTypes.java

-    public static final LogAxis NONE = DummyObjectProvider.createFor(LogAxis.class, "NONE");
-
-    public static final LogAxis X = DummyObjectProvider.createFor(LogAxis.class, "X");
+    public static final VariantQueryType<Material> MATERIAL = DummyObjectProvider.createFor(VariantQueryType.class, "MATERIAL");


With my comment above about Variety vs Material, this is an example where "Variants" will win overall as you can use different varieties to describe something unique (almost mergeable with properties).

gabizou · 2017-12-07T07:03:18Z

src/main/java/org/spongepowered/api/variant/VariantQueryableCatalogType.java

+
+import java.util.Optional;
+
+public interface VariantQueryableCatalogType extends CatalogType {


Description of what this type of catalog type does?

gabizou · 2017-12-07T07:03:52Z

src/main/java/org/spongepowered/api/variant/VariantQueryableCatalogType.java

+     * @param <T> The argument type
+     * @return The argument type, if the query type is supported
+     */
+    <T> Optional<T> getArgument(VariantQueryType<T> queryType);


Why is it a query type instead of just VariantQuery? We don't call Keys KeyType.

It was to return the argument you would pass in to create the VariantQuery from the VariantQueryType.

Cybermaxke · 2017-12-07T16:27:29Z

@gabizou I will open a new PR that targets API 7 for the Variety system. I will try to reuse things from the Data API that are already present to replace the VariantQuery, like using properties to query for specific things.
TreeTypeProperty, DyeColorProperty, etc.
Which other properties would you think that would be worth adding, based from the data that will be removed, for example:
GoldenAppleTypeProperty, CoalTypeProperty, etc.

kashike · 2017-12-08T20:42:02Z

Please rebase against 1.13.

ryantheleach · 2017-12-11T00:43:36Z

src/main/java/org/spongepowered/api/data/key/Keys.java

+     *
+     * @see LitData#lit()
+     */
+    public static final Key<Value<Boolean>> LIT = KeyFactory.fake("LIT");


gabizou

I’m willing to allow keeping the removed data, only to support retrieving the data, and then removing it later. This way plugins still will have an upgrade understanding. I know that this api is supposed to break the world, but it’d be nice to retain it for support like we did with some data types in api 6.

Cybermaxke · 2018-02-15T21:05:22Z

@gabizou I don't feel like keeping something as half working is a good idea. Maybe it would be better to provide a spongedocs page dedicated to updating from API7 to API8, related to big changes like item types/block types?

parlough · 2018-02-15T21:12:08Z

@Cybermaxke I think I share the same opinion, keeping it with half functionality will unnecessarily clutter the API and only confuse developers. You are right though we need to document all of the changes well, not only updating outdated docs but a migration guide will probably be highly beneficial. In general we should keep a more detailed changelog of larger changes as well.

liach · 2018-03-20T15:22:45Z

Bump: @Cybermaxke Would you add contents from the aquatic update, aside technical changes?

Cybermaxke · 2018-03-20T16:33:26Z

@liach Yes, I am planning on doing that.

gabizou · 2018-04-12T07:29:28Z

Can you update the PR description as to what is going on? (I believe we opted to support legacy data, on the basis that it was read only, much like the option we took with HorseVariants when horses got split up.

gabizou

There's a metric ton of formatting changes that I'm not able to see what is actually added/changed in the API without scrolling and reading every line diff (I feel that the first 10th of the PR changes are just formatting....)

Cybermaxke · 2018-04-12T10:49:14Z

@gabizou I rebased against bleeding but didn't change the target branch, is fixed now.

I still think that keeping half broken API isn't a good thing, and that it would be better to provide good documentation to transition between API 7 and API 8 (once 1.13 is merged).

Would love some input on this from some other developers

parlough · 2018-04-12T15:31:55Z

@Cybermaxke I share your opinion, leaving half functional API would be a pain for maintenance and it will just cause confusion for users. They will have to update and make changes anyway for their plugin to be functional and they can update their removed data retrievals while they're at it. Leaving it will have them expecting API to work that won't work and/or even can't be made to work.

Our best solution is documenting all the necessary changes quite well, through written documentation, but also through video or stream. I am planning to work on this quite a bit and I know others will be happy to help.

kashike · 2018-06-05T09:54:42Z

...main/java/org/spongepowered/api/data/manipulator/immutable/block/ImmutableDecayableData.java

+     * @return The immutable value for the persistent state
+     * @see Keys#PERSISTS
+     */
+    ImmutableValue<Boolean> persists();


persistent

@kashike I named it persists to match the Key

name the key persistent, then! :P

Cybermaxke · 2019-01-19T15:56:26Z

Moved to #1949

Cybermaxke requested review from Faithcaio and gabizou as code owners November 27, 2017 11:59

ryantheleach added this to the Revision 8.0 milestone Nov 27, 2017

ryantheleach added status: input wanted status: wip api: 8 (u) version: 1.16 (unsupported since Oct 17th 2023) labels Nov 27, 2017

killjoy1221 mentioned this pull request Nov 28, 2017

[API8] Add datapack support #1706

Closed

Cybermaxke changed the base branch from 1.13 to bleeding December 4, 2017 16:50

Cybermaxke force-pushed the 1.13 branch from 34a5ee7 to d8fca68 Compare December 4, 2017 16:52

gabizou requested changes Dec 7, 2017

View reviewed changes

kashike changed the base branch from bleeding to 1.13 December 8, 2017 20:37

kashike force-pushed the 1.13 branch from fdc5328 to d7f2e69 Compare December 8, 2017 20:41

Cybermaxke force-pushed the 1.13 branch from acb4011 to 7670f60 Compare December 9, 2017 09:03

Cybermaxke requested a review from Deamon5550 as a code owner December 9, 2017 16:47

ryantheleach reviewed Dec 11, 2017

View reviewed changes

Cybermaxke mentioned this pull request Dec 11, 2017

Bed data (Which half + Direction of other half) + More generic way to get blocks of multi-block structure #1492

Open

Cybermaxke mentioned this pull request Dec 20, 2017

Minecraft 1.13 - The Technical + Aquatic Update LanternPowered/Lantern#48

Closed

kashike force-pushed the 1.13 branch 2 times, most recently from 8277fda to d32cbb1 Compare January 8, 2018 03:30

Cybermaxke force-pushed the 1.13 branch 2 times, most recently from 060576b to 06647ca Compare January 15, 2018 14:05

gabizou self-assigned this Jan 15, 2018

Cybermaxke mentioned this pull request Jan 17, 2018

Minecraft 1.13 - Refactor statistics #1742

Closed

2 tasks

gabizou requested changes Jan 19, 2018

View reviewed changes

Cybermaxke changed the title ~~1.13 - The Technical Update~~ 1.13 - The Technical and Aquatic Update Mar 20, 2018

Cybermaxke force-pushed the 1.13 branch from e02c089 to 4617482 Compare April 7, 2018 15:54

Cybermaxke requested review from kashike, stephan-gh and Zidane as code owners April 7, 2018 15:54

gabizou requested changes Apr 12, 2018

View reviewed changes

Cybermaxke changed the base branch from 1.13 to bleeding April 12, 2018 10:43

Cybermaxke force-pushed the 1.13 branch from 47d3ace to 728ce67 Compare May 12, 2018 16:51

Cybermaxke force-pushed the 1.13 branch from 1e30f69 to fa93d0f Compare May 26, 2018 08:08

kashike suggested changes Jun 5, 2018

View reviewed changes

Cybermaxke force-pushed the 1.13 branch from a057bcf to dcfe5ef Compare June 22, 2018 12:13

gabizou added the branch: bleeding label Aug 30, 2018

Cybermaxke force-pushed the 1.13 branch from f28307a to 7fdf6e1 Compare September 20, 2018 15:23

Update for Minecraft 1.13.1

be904f8

Cybermaxke force-pushed the 1.13 branch from 7fdf6e1 to be904f8 Compare September 20, 2018 15:26

Rename SILVER DyeColor to LIGHT_GRAY.

d968361

Deamon5550 removed their request for review September 21, 2018 00:03

Cybermaxke closed this Jan 19, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1.13 - The Technical and Aquatic Update #1704

1.13 - The Technical and Aquatic Update #1704

Cybermaxke commented Nov 27, 2017 •

edited

VapidLinus commented Nov 27, 2017 •

edited

me4502 commented Nov 27, 2017

ryantheleach commented Nov 27, 2017

Cybermaxke commented Nov 27, 2017

Cybermaxke commented Nov 27, 2017

gabizou left a comment •

edited

gabizou Dec 7, 2017

gabizou Dec 7, 2017

JBYoshi Dec 9, 2017

gabizou Dec 7, 2017

gabizou Dec 7, 2017

gabizou Dec 7, 2017

gabizou Dec 7, 2017

Cybermaxke Dec 7, 2017

Cybermaxke commented Dec 7, 2017

kashike commented Dec 8, 2017

ryantheleach Dec 11, 2017

gabizou left a comment

Cybermaxke commented Feb 15, 2018

parlough commented Feb 15, 2018

liach commented Mar 20, 2018

Cybermaxke commented Mar 20, 2018

gabizou commented Apr 12, 2018

gabizou left a comment

Cybermaxke commented Apr 12, 2018 •

edited

parlough commented Apr 12, 2018

kashike Jun 5, 2018

Cybermaxke Jun 5, 2018

kashike Jun 5, 2018

Cybermaxke commented Jan 19, 2019

		@@ -207,6 +207,8 @@

		public static final SoundType BLOCK_PORTAL_TRIGGER = DummyObjectProvider.createFor(SoundType.class, "BLOCK_PORTAL_TRIGGER");

		public static final SoundType BLOCK_PUMPKIN_CARVE = DummyObjectProvider.createFor(SoundType.class, "BLOCK_PUMPKIN_CARVE");


		import java.util.Optional;

		public interface VariantQueryableCatalogType extends CatalogType {

1.13 - The Technical and Aquatic Update #1704

1.13 - The Technical and Aquatic Update #1704

Conversation

Cybermaxke commented Nov 27, 2017 • edited

Modify/remove:

New/Modified

VapidLinus commented Nov 27, 2017 • edited

me4502 commented Nov 27, 2017

ryantheleach commented Nov 27, 2017

Cybermaxke commented Nov 27, 2017

Cybermaxke commented Nov 27, 2017

gabizou left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cybermaxke commented Dec 7, 2017

kashike commented Dec 8, 2017

Choose a reason for hiding this comment

gabizou left a comment

Choose a reason for hiding this comment

Cybermaxke commented Feb 15, 2018

parlough commented Feb 15, 2018

liach commented Mar 20, 2018

Cybermaxke commented Mar 20, 2018

gabizou commented Apr 12, 2018

gabizou left a comment

Choose a reason for hiding this comment

Cybermaxke commented Apr 12, 2018 • edited

parlough commented Apr 12, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cybermaxke commented Jan 19, 2019

Cybermaxke commented Nov 27, 2017 •

edited

VapidLinus commented Nov 27, 2017 •

edited

gabizou left a comment •

edited

Cybermaxke commented Apr 12, 2018 •

edited