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

[draft] Allow Inventory Manager to list, pull, and push to player Curios slots #480

Draft
wants to merge 1 commit into
base: release/1.18
Choose a base branch
from

Conversation

Kurtoid
Copy link

@Kurtoid Kurtoid commented Jul 2, 2023

@SirEndii
Copy link
Member

SirEndii commented Jul 2, 2023

One thing I noticed is, that you don't check if curios is loaded since it's a optional dependency

So if curios is not loaded, the game would crash when someone runs these functions

@SirEndii
Copy link
Member

SirEndii commented Jul 2, 2023

And ignore the failed github action, these currently do not work for pull requests

@SirEdvin
Copy link
Collaborator

SirEdvin commented Jul 2, 2023

I can suggest reusing peripheral plugin logic as environmental sensor do.

@Kurtoid
Copy link
Author

Kurtoid commented Jul 2, 2023

One thing I noticed is, that you don't check if curios is loaded since it's a optional dependency

Would implementing this as a plugin handle that? I'll give it a try

@Kurtoid
Copy link
Author

Kurtoid commented Jul 2, 2023

Making sure I understand correctly: I should move the Curios calls to a new class in common/addons/curios/, create a PERIPHERAL_PLUGINS like in the Environmental Sensor, and define and use InventoryManagerPeripheral.addIntegrationPlugin ?

Edit: wait, BasePeripheral has addPlugin - should I use that, or only call addPlugin at initialization like the Environmental Sensor? For now, I'll follow what the Environmental Sensor does as recommended by SirEdvin

@Kurtoid
Copy link
Author

Kurtoid commented Jul 2, 2023

Just encountered this: TheIllusiveC4/Curios#238
so this might not even be possible unless there's a workaround

@SirEndii
Copy link
Member

SirEndii commented Jul 2, 2023

Just encountered this: TheIllusiveC4/Curios#238 so this might not even be possible unless there's a workaround

Oh indeed, that's a issue

A workaround would be that we check the items before adding them

@SirEndii
Copy link
Member

SirEndii commented Jul 2, 2023

Edit: wait, BasePeripheral has addPlugin - should I use that, or only call addPlugin at initialization like the Environmental Sensor? For now, I'll follow what the Environmental Sensor does as recommended by SirEdvin

Yeah, do it like edvin recommended it

@SirEndii
Copy link
Member

SirEndii commented Jul 7, 2023

Any progress? I don't want to put you under pressure or give you time pressure
I'm just asking because I'll release an update soon and want to know if I'll include your changes or not

@Kurtoid
Copy link
Author

Kurtoid commented Jul 7, 2023 via email

@SirEndii
Copy link
Member

SirEndii commented Jul 7, 2023

Alright, no problem
And thank you for the quick response

@SirEndii SirEndii added enhancement New feature or request 1.18x labels Jul 19, 2023
@SirEndii
Copy link
Member

Greetings @Kurtoid
Do you have any plans to work on this?

@Kurtoid
Copy link
Author

Kurtoid commented Apr 16, 2024

Whoops, I took a break from minecraft and forgot about this. It looks like Curios now has the support needed to do this without workarounds (TheIllusiveC4/Curios#238 (comment)). I'm not sure if I can do it in a timely manner, though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.18x enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Curios compatibility for the Inventory Manager
3 participants