Skip to content

[API8] Add datapack support#1706

Closed
mattmess1221 wants to merge 22 commits into
SpongePowered:api-8from
mattmess1221:feature/packs
Closed

[API8] Add datapack support#1706
mattmess1221 wants to merge 22 commits into
SpongePowered:api-8from
mattmess1221:feature/packs

Conversation

@mattmess1221
Copy link
Copy Markdown
Contributor

@mattmess1221 mattmess1221 commented Nov 28, 2017

I'm assuming the client's resource manager was reused for the server. Likely more edits will come when I have access to deobfuscated code.

@kashike kashike added status: input wanted status: needs review api: 8 (u) version: 1.16 (unsupported since Oct 17th 2023) labels Nov 28, 2017
@liach
Copy link
Copy Markdown
Contributor

liach commented Nov 29, 2017

should we merge resource path with catalog key?

@mattmess1221
Copy link
Copy Markdown
Contributor Author

If we do, we should settle on a better suited name. It doesn't make sense to get a Resource from a CatalogKey, and neither does a CatalogType with ResourcePath

Though they are slightly different. While both have a namespace and value, the resource path can contain a forward slash / and dot .. If anything, it should have a common parent-interface indicating it can have a namespace.

@liach
Copy link
Copy Markdown
Contributor

liach commented Nov 29, 2017

Something like BinomialName for a parent interface?

@mattmess1221
Copy link
Copy Markdown
Contributor Author

I was thinking it would include the word namespace. Something like NamespacedIdentifier

@liach
Copy link
Copy Markdown
Contributor

liach commented Nov 29, 2017

SpacedName

Comment thread src/main/java/org/spongepowered/api/Game.java Outdated
@ryantheleach
Copy link
Copy Markdown
Contributor

ryantheleach commented Jan 4, 2018

I don't like that this outright deprecates the Asset API.

The goals of the Asset API were to simplify the process for new programmers to extract resources from their jar files, to their plugin directories on disk, usually used for extracting default or example configurations.

If this is to happen, I'd like at least a little attention paid to giving those people a clear migration path, and maybe some docs to go along with it to explain exactly what this is and how it works.

I also think the name DataPack is pretty close to the Mojang defined term.

If this isn't intended to be directly compatible with that, or interact with that in some way then it's poorly named.

But maybe I'm just not following why the name change is needed?

Comment thread src/main/java/org/spongepowered/api/resource/Resource.java Outdated
Comment thread src/main/java/org/spongepowered/api/Sponge.java Outdated
Comment thread src/main/java/org/spongepowered/api/resource/InputStreamSupplier.java Outdated
Comment thread src/main/java/org/spongepowered/api/resource/Resource.java Outdated
Comment thread src/main/java/org/spongepowered/api/resource/ResourcePath.java
Comment thread src/main/java/org/spongepowered/api/resource/Pack.java Outdated
Comment thread src/main/java/org/spongepowered/api/resource/Pack.java Outdated
@Cybermaxke
Copy link
Copy Markdown
Contributor

Please keep the ResourcePath separate from the CatalogId. They represent two different things and shouldn't be merged as one. They also have different contraints.

Comment thread src/main/java/org/spongepowered/api/resource/ResourceManager.java Outdated
Comment thread src/main/java/org/spongepowered/api/resource/ResourceManager.java Outdated
String getPath();

@Override
int hashCode();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this in the API?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For arbitrary order?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it there originally because I wanted it to be clear that this can be used in a Map. Now I'm just saying so in the docs.

Comment thread src/main/java/org/spongepowered/api/resource/ResourceProvider.java
Comment thread src/main/java/org/spongepowered/api/resource/PackProvider.java Outdated
Comment thread src/main/java/org/spongepowered/api/resource/ResourcePath.java Outdated
Comment thread src/main/java/org/spongepowered/api/resource/ResourceId.java Outdated
Comment thread src/main/java/org/spongepowered/api/event/resource/ResourceReloadEvent.java Outdated
Comment thread src/main/java/org/spongepowered/api/resource/Resource.java Outdated
@mattmess1221 mattmess1221 force-pushed the feature/packs branch 2 times, most recently from 4a4e07c to 1ad90b3 Compare January 8, 2018 03:20
@Zidane
Copy link
Copy Markdown
Member

Zidane commented Jun 28, 2020

@killjoy1221 Need this finished up soon else it won't make it for API 8.0.0

@mattmess1221
Copy link
Copy Markdown
Contributor Author

#2135

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: 8 (u) version: 1.16 (unsupported since Oct 17th 2023) status: input wanted status: needs review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants