Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NO MERGE] Contextual Data API #2496

Draft
wants to merge 2 commits into
base: api-11
Choose a base branch
from
Draft

[NO MERGE] Contextual Data API #2496

wants to merge 2 commits into from

Conversation

aromaa
Copy link
Member

@aromaa aromaa commented Apr 23, 2024

Sponge | SpongeAPI

Open questions?

  • Server as DataPerspective? One could implement way to hide specific entity data like health from clients globally.
  • This could also be used to change the world properties send to the client like weather by taking advantage of the World -> Entity data direction. Do we want that?
  • How do we implement collection operations? What happen happen when you do offerSingle to the data perspective and the base entity already has elements. Append? Create new list with just that single item?
  • Do we want to allow plugins to provide more info how to solve merge conflicts?
    • Provide types to provide the operation type? Like Value<Integer> could have addition, multiplication, etc.
    • Allow that same concept for values with collections?


Iterable<DataPerspective> perceives();

ValueContainer getDataPerception(DataPerspective perspective);
Copy link
Member

Choose a reason for hiding this comment

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

Should just be dataPerception(...) no?

import org.checkerframework.checker.nullness.qual.Nullable;
import org.spongepowered.api.data.value.Value;

public interface DataPerspectiveResolver<V extends Value<E>, E> {
Copy link
Member

Choose a reason for hiding this comment

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

Is this interface meant to be implemented? (DataRegistration below gives that impression).

If so, maybe it is best to make this abstract? I do not like a method that returns the key that isn't enforced to always return the same key (the instance could change).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is at the moment required to light up contextual data, otherwise we refuse to apply the data. I followed the same style we use for DataProvider already, that also has getter for the key.

But it might be more beneficial to try to intergrade this whole concept to be part of DataProvider to make them always be contextual aware, but that then touches a lot of core data API surface area.

Copy link
Member

Choose a reason for hiding this comment

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

it's up to you as it's a commitment on your part but if you are up to it, I think this is a spot where we go all in and make DataProvider and the data API as a whole contextual aware. It was always meant to be, no one sat down to do it was always the issue.

Until now :).

@aromaa aromaa changed the title [NO MERGE] Contextual API [NO MERGE] Contextual Data API Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants