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

Pull bookmarked items from container to player inventory #410

Merged
merged 11 commits into from
Aug 7, 2023

Conversation

Nilau1998
Copy link

This PR includes the following features:

  • A new button/function to pull all bookmarks which are on the current bookmarks page into the players inventory from a given open container.
  • A new API within NEI to realise this feature.

Explanation as to why this API was nessessary:

A container has to be registered within the API. The main reason behind this is, that containers might works differently from each other. Therefore it is also nessessary to implement a container IBookmarkContainerHandler which handles how the items are being moved from the registered container to the players inventory. The handler expects the open gui, and an ArrayList of the bookmarked items.
The main reason for this, is that containers differ alot in some cases, like vanilla chests and AE2 Terminal windows. Vanilla chests can be handled by simply moving the items via simulation of clicks. With AE2 a packet has to be send to retreive items from the entire ME Network.

A simple video showcasing this feature for vanilla chests:
https://www.youtube.com/watch?v=70NIA_PrlbE
https://github.com/GTNewHorizons/NotEnoughItems/assets/64710705/e07c8be3-bae5-4e08-a387-628387d53c93

@Dream-Master Dream-Master requested review from a team August 5, 2023 16:01
Copy link
Member

@glowredman glowredman left a comment

Choose a reason for hiding this comment

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

Some minor things, which I noticed. This is not a full review, someone else should still look at this aswell.

src/main/java/codechicken/nei/BookmarkContainerInfo.java Outdated Show resolved Hide resolved
src/main/java/codechicken/nei/BookmarkPanel.java Outdated Show resolved Hide resolved
@Dream-Master
Copy link
Member

@Nilau1998

@Nilau1998
Copy link
Author

@glowredman I commited your suggestions, I only made my class attribute private now and didn't touch RecipeInfo.java incase it breaks other things somewhere down the chain.

@glowredman glowredman dismissed their stale review August 7, 2023 10:55

Requested changes have been made

@glowredman glowredman requested review from a team and removed request for glowredman August 7, 2023 10:55
@Dream-Master Dream-Master merged commit 00c4411 into GTNewHorizons:master Aug 7, 2023
1 check passed
@Dream-Master
Copy link
Member

oh didnt see it was a future freeze pr. It has no tag and i dont see the project on the side. I add now a new tag for nei. Should i remove it to prevent daxxl in add it next release ? Or we have enough time to test it to be sure we not break things ?

@miozune
Copy link
Member

miozune commented Aug 8, 2023

I don't think it can break existing features, but if you want to be sure you can roll back version in daxxl. Removing tag is bad in general.

@Nilau1998
Copy link
Author

Nilau1998 commented Aug 8, 2023

@Dream-Master Personally I wouldn't deliever this feature with the upcoming stable version. The feature does work, but only for vanilla chests which ofc is very limiting for the user.

I'd prefer it, if you do what miozune said and use a previous version without this feature. It gives me more time to implement the API in other mods that have storage systems and therefore make the player more happy in the end.

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.

4 participants