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

Make StructureModifier Iterable #2477

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

BradBot1
Copy link
Contributor

This will simply allow for for loops to be used on StructureModifier.

See below for example syntax this will enable:

@Override
public void onRecievePacket(final PacketEvent packetEvent) {
  final PacketContainer packet = packetEvent.getPacket();
  for(final ItemStack itemStack : packet.getItemModifier()) {
    // Do something with the item
  }
}

@dmulloy2
Copy link
Owner

code looks good. quick question: is it possible to incorporate writes somehow? looks like all you can do is read

@BradBot1
Copy link
Contributor Author

If there was a staging Object that had a read and write method then definitely, but iterators are not really made for it themselves

I can get one made in a couple seconds if you'd like

@BradBot1
Copy link
Contributor Author

I've just completed how I'd expect it to work with read and write access, the new setup would be as follows:

@Override
public void onRecievePacket(final PacketEvent packetEvent) {
  final PacketContainer packet = packetEvent.getPacket();
  for(final StructureModifierIntermediate<ItemStack> intermediate : packet.getItemModifier()) {
    final ItemStack itemStack = intermediate.get(); // read the item
    // Do something with the item
    intermediate.set(itemStack); // write to the item
  }
}

Copy link
Contributor

@derklaro derklaro left a comment

Choose a reason for hiding this comment

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

I like to suggest to override spliterator as well and returning a new spliterator like:

  • Giving in the size of the structure modifier
  • Characteristics:
    • DISTINCT: we will not encounter the same modifier for a structure twice.
    • IMMUTABLE
    • NONNULL
    • ORDERED: the elements will be in the same order as the fields are defined in the source class
    • SIZED: can be added, but when using Spliterators.spliterator it will be added automatically

And if we're on it we can also add a stream getter to the class.

@BradBot1
Copy link
Contributor Author

Since the Spliterator type must be of the same as the Iterator it cannot be IMMUTABLE without the Iterator also being so ;/

@BradBot1
Copy link
Contributor Author

And for streams: I don't see the utility in them here as the use cases where they could be employed are very limited.

The consumer can also create their own if need be via:

StructureModifier<?> structureModifier;
Steam<?> stream = StreamSupport.stream(structureModifier.spliterator());

@derklaro
Copy link
Contributor

I'm not sure what you're trying to say...
Also not sure if there is a reason, but you can use Spliterators.spliterator(...) instead of implementing it yourself

@derklaro
Copy link
Contributor

And for streams: I don't see the utility in them here as the use cases where they could be employed are very limited.

The use cases are only limited until you expose the underlying StructureModifier from StructureModifierIntermediate, or at least, expose information about the wrapped structure modifier

@BradBot1
Copy link
Contributor Author

And for streams: I don't see the utility in them here as the use cases where they could be employed are very limited.

The use cases are only limited until you expose the underlying StructureModifier from StructureModifierIntermediate, or at least, expose information about the wrapped structure modifier

Would you mind expressing with some pseudocode what you mean by this? I'm still not grasping what you are trying to say

@BradBot1
Copy link
Contributor Author

I'm not sure what you're trying to say... Also not sure if there is a reason, but you can use Spliterators.spliterator(...) instead of implementing it yourself

Because you asked for it? The default implementation does exactly that

@derklaro
Copy link
Contributor

Would you mind expressing with some pseudocode what you mean by this? I'm still not grasping what you are trying to say

Well if information about the strucutred modifier is exposed (for example by adding a getter for it from the Intermediate class) to the consumer this would allow f. ex. flat mapping to get all nested fields, filtering by field type etc.

Because you asked for it? The default implementation does exactly that

I'm just confused why you implemented the spliterator rather than using Spliterators.spliterator(iterator, size, ...) from the existing apis instead...

@BradBot1
Copy link
Contributor Author

For the spliterator what's done is done, I can roll back and use the API if you would prefer that

@derklaro
Copy link
Contributor

No need to roll back, just delete your implementation and use the Spliterators static method instead 😄

@dmulloy2 dmulloy2 requested a review from derklaro August 5, 2023 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants