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

NEI Bookmark Pulling for AE2 #372

Merged
merged 7 commits into from
Sep 24, 2023

Conversation

Nilau1998
Copy link

Continuation of the Bookmark Pulling API implemented in a previos PR (GTNewHorizons/NotEnoughItems#410 (review)). This time we are getting the same logic but for AE2 Panels and the Skystone Chests.

The Skystone chests sadly needed a bunch of logic implemented to get the movement by simulating player clicks. If there is any other way I'll gladly implement it using that, I couldn't find anything to simply move items in the the default AE2 code however which is why I feel back to the TransferManager from NEI for inspiration.

Regular Terminals use the packet system to request items from the ME System and transfer them into the players inventory.

Showcase this implementation where a player would want to get all components for a Fusion Reactor from the Terminals and some Steam machinery from Skychests.

https://youtu.be/cJofSS__P_s

@Dream-Master Dream-Master requested review from a team August 7, 2023 18:59
@firenoo firenoo added the ongoing freeze - do not merge Not just a bug fix and thus affected by a current freeze for a upcoming version label Aug 7, 2023
@Nilau1998 Nilau1998 requested a review from firenoo August 9, 2023 12:14
@firenoo firenoo removed the ongoing freeze - do not merge Not just a bug fix and thus affected by a current freeze for a upcoming version label Sep 4, 2023
@Nilau1998 Nilau1998 requested a review from a team as a code owner September 6, 2023 19:16
@firenoo firenoo force-pushed the NEI_Bookmark_Pulling branch 2 times, most recently from 1b55bc6 to 3c00e10 Compare September 7, 2023 15:38
@Nilau1998
Copy link
Author

Don't merge yet, I forgot to test the ME Chest ingame after registering it in the API.

@Nilau1998
Copy link
Author

Okay the feature does work for ME chests! Sorry for the delay :v)

@Dream-Master
Copy link
Member

@Nilau1998 is the pr done? i ask Firenoo and he said no.

@Nilau1998
Copy link
Author

@firenoo what are we missing? I implemented the points you mentioned, see 1c5a7e4.

@firenoo
Copy link

firenoo commented Sep 13, 2023

Very sorry, I was tired that time and read it "does not" work with ME chests so I was waiting to see if it would be fixed. I'll do some QA when I have time

@firenoo
Copy link

firenoo commented Sep 18, 2023

Going classify this as a bug. I have 2 stacks of stone in my bookmarks, and my inventory is full of stone 1x stacks. I pull, and get only 126 instead of the full 128. This happens with other stack amounts; it only chooses 2 slots to fill,

@firenoo
Copy link

firenoo commented Sep 18, 2023

Bug above is true when pulling 2 types, for each type

@firenoo
Copy link

firenoo commented Sep 24, 2023

Will probably need to rebase again...

@Dream-Master
Copy link
Member

Will probably need to rebase again...

What is wrong? Why rebase

@firenoo
Copy link

firenoo commented Sep 24, 2023

i dont want to see stuff from the main branch on side branches

@Dream-Master
Copy link
Member

ah ok clean history

@Dream-Master
Copy link
Member

@firenoo but beside this is the pr now ok ?

Copy link

@firenoo firenoo left a comment

Choose a reason for hiding this comment

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

Bugs fixed

add forgotten import
Copy link

@firenoo firenoo left a comment

Choose a reason for hiding this comment

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

Works fine. Thanks for following through

@Dream-Master Dream-Master merged commit 1564929 into GTNewHorizons:master Sep 24, 2023
1 check passed
@Nilau1998 Nilau1998 deleted the NEI_Bookmark_Pulling branch September 30, 2023 12:27
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.

3 participants