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

OpenPeripheral, Plethora and the future #452

Closed
SquidDev opened this issue May 18, 2020 · 7 comments
Closed

OpenPeripheral, Plethora and the future #452

SquidDev opened this issue May 18, 2020 · 7 comments
Labels
area-Minecraft This affects CC's Minecraft-specific content. enhancement An extension of a feature or a new feature. feedback wanted Tell me what you want, what you really really want. version-1.15.x Bugs affecting the Forge 1.15.x version.

Comments

@SquidDev
Copy link
Member

Background

Historically, CC has never had much integration with other mods. That has changed more recently, with support for JEI, MCMultipart, CraftTweaker and Charset.

However, what hasn't changed is that CC:T does not provide peripheral methods for external mods (or even Vanilla!). This has instead been provided by other mods, such as OpenPeripheral and Plethora. This is fine, but not without its problems:

  • It's confusing: Most people aren't aware that these methods come from other mods (see Can't place any modems on Mekanism energy cells #449, and numerous questions on Discord and Reddit).
  • We need to wait on other mods to update: Both Plethora and OP added gameplay features, which add an additional burden to updating the mod.

This is, in short, a Problem.

Proposal

I think the "best" solution for now is to merge the vanilla (and Forge) integration into CC: Tweaked. This means we don't add any additional dependencies (which make updating the mod harder), but should provide enough functionality for most users.

Effectively, this means the following changes:

  • Merge some of Plethora's API into CC: Tweaked.
  • Port over inventory, fluid and energy methods, and some vanilla/CC meta providers.
  • All remaining mod-integration code will stay in Plethora.

There's a lot of stuff about how the API should look which really needs to be hashed out before doing any work on this. Plethora has a lot of nice functionality, but it ends up being stupidly complex and somewhat broken (I'm looking at you converters).

@SquidDev SquidDev added enhancement An extension of a feature or a new feature. feedback wanted Tell me what you want, what you really really want. area-Minecraft This affects CC's Minecraft-specific content. version-1.15.x Bugs affecting the Forge 1.15.x version. labels May 18, 2020
@SquidDev
Copy link
Member Author

Yes, I realise there's nothing really actionable right now. I got halfway through writing this issue, and then realised that everything else was just going to be a list of problems with Plethora and me not having a clue on how to fix them. That can probably wait 'til later.

@Lemmmy
Copy link
Contributor

Lemmmy commented May 18, 2020

I agree with moving some of Plethora's basic features to CC:T. It always seemed to make sense to me that inventory interaction should be a native CC feature, given the sheer amount of things made possible by it. One thing that comes to mind, though, is the introspection module - I'm imagining you don't plan on bringing that or any of the other modules over, but it provides some basic functionality for turtles that wouldn't be available otherwise.

@SquidDev
Copy link
Member Author

I think the only the introspection module can do that turtles can't is getting complete item metadata?

Ideally getItemDetails would include that. However, it currently runs on the computer thread, which doesn't play well with meta providers.

@Lemmmy
Copy link
Contributor

Lemmmy commented May 18, 2020

Yeah that sounds about right. The only other thing that makes sense to me is adding another method turtle.getItemMeta() which provides details + Plethora's meta providers, and slap some warnings on it saying its expensive so use it as little as possible, and existing code won't have any additional overhead, but then we'd have way too many turtle methods

@Lupus590
Copy link
Contributor

could the extra metadata be fetched with an extra arg to getItemDetails?

@Lemmmy
Copy link
Contributor

Lemmmy commented May 21, 2020

That does sound a lot better

SquidDev added a commit that referenced this issue Jun 27, 2020
This exposes a basic peripheral for any tile entity which does not have methods
already registered. We currently provide the following methods:

 - Inventories: size, list, getItemMeta, pushItems, pullItems.
 - Energy storage: getEnergy, getEnergyCapacity
 - Fluid tanks: tanks(), pushFluid, pullFluid.

These methods are currently experimental - it must be enabled through 
`experimental.generic_peripherals`. While this is an initial step towards
implementing #452, but is by no means complete.
SquidDev added a commit that referenced this issue Jun 30, 2020
 - Refer to this as "data" rather than "metadata". I'm still not sure
   where the meta came from - blame OpenPeripheral I guess.
 - Likewise, use getItemDetail within inventory methods, rather than
   getItemMeta.
 - Refactor common data-getting code into one class. This means that
   turtle.getItemDetail, turtle.inspect and commands.getBlockInfo all
   use the same code.
 - turtle.getItemDetail now accepts a second "detailed" parameter which
   will include the full metadata (#471, #452).
 - Tags are now only included in the detailed list. This is a breaking
   change, however should only affect one version (1.89.x) and I'm not
   convinced that the previous behaviour was safe.
@SquidDev SquidDev pinned this issue Sep 7, 2020
@prozacgod
Copy link

prozacgod commented Sep 18, 2020

This is a great feature idea! In the past, I've ran a modpack for myself and some friends that's a "computercraft/opencomputers" focused world. We have some tech mods (Integrated Dynamics, is probably the most advanced) but not a lot of advanced stuff can be done without programming! (It's super fun)

I can't recall which of the cc interface apis did this - the old open blocks one and the newer plethora one - but one of the gave us quite a lot more meta data than the other. (maybe not the full NBT but close in some cases!)

Now, this is our little sandbox, we REALLY want the ability to get full meta data on stuff in chests, or blocks - But we also know this could be a bit cheaty in some modpack/ or normal gameplay. I'd love for there to at least be an option to get "deep" meta data - regardless of how slow it may be, we can code around that just fine.

But for game balance, pack authors - I'd like to say that a blacklist/whitelist be available for blocking meta data other than bare minimum for some items, for pack authors to decide if knowing the exact amount of mana in a botania mana pool is cheating (A comparator output could always be used)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Minecraft This affects CC's Minecraft-specific content. enhancement An extension of a feature or a new feature. feedback wanted Tell me what you want, what you really really want. version-1.15.x Bugs affecting the Forge 1.15.x version.
Projects
None yet
Development

No branches or pull requests

4 participants