ComputerCraft

ComputerCraft

21M Downloads

Allow multiple peripherals on the same block

Vexatos opened this issue ยท 30 comments

commented

This has been done in almost every decently-sized CC addon before, using various more-or-less hacky implementations. Now that we have the source, it should be a good idea to have it in the mod itself.

Currently, only one peripheral can exist on a block at any time; if two providers would return a peripheral for any given coordinate, the one registered first is chosen and all the others are ignored. Of course, this becomes silly quickly as soon as you add CC addons to your game (think about OpenPeripheral which can add a peripheral to every single inventory in the game). Now, the idea is to modify the peripheral system so that all peripherals returned for any given position are used, and the methods of all of them being made available. There will need to be a new method (maybe for compat reasons on a new interface? Peripherals without would simply have a default priority of 0) returning some sort of peripheral priority (with the higher number being the higher priority), so that when two peripherals both provide a method with the same name, the one with the higher priority will be chosen. The highest-priority peripheral would also determine the peripheral's general name.

I have already written such a system for CC before, you can find it here (most importantly, this class). You could implement it in a very similar and much cleaner way in CC directly.
As I said, I know other mods have done a very similar thing before, so I would like to see suggestions about how to do this in the best possible way. Maybe we can find a solution that works for most addon devs.

commented

Hmm.. Looking at this suggestion it would require redesigning how for of peripheral functions work. Like wrap per example. Soo i peripheral.wrap("left") and left has 2 periphrals in one block. Witch one is wraped now? Your suggestion makes it combine both into one peripheral? Wouldn't it make more sense to keep them separated and make player choose witch one to use? That way you avoid the name conflict entirely.

It would require some good planning to fit the contributing rules.

commented

The idea is indeed that both are wrapped into a single one. The name of the merged peripheral would be the name of the peripheral with the highest priority. This would not change CC's Lua side and thus wouldn't introduce any additional complexity for players. This does mean that the peripheral name might change if you add a new CC addon to your mod pack, but it should be expected that adding mods changes behaviours.

commented
commented
commented

Merging peripherals would also allow changing behaviour on methods under certain circumstances. For instance, all RF-using devices might have a method getEnergyStored, but certain blocks like the EnderIO capacitor bank need a special implementation of that method instead of the generic one provided to all blocks. With the merging system, one could simply create a peripheral specifically for that block but with a higher priority than the generic peripheral, overriding getEnergyStored and thus making it return a correct value without causing the confusion of two different getEnergyStored methods being available (one of which doesn't work on this specific block).

This example is what Computronics currently does interacting with the more generic method provided by OpenPeripheral-Integration.

commented

Well if it went with "left", "left2", "left3" then you jst use the peripheral.getType(string side) and use correct handler in your code to handle correct peripheral.

If its merged you have no idea from witch peripheral function is coming from and if it will error in your handling or not,

My vote is for first of Dan200 ideas: - "left", "left2", "left3". Makes sense and fits current ideology.

commented

A more generic getType("left") needs to return a table then, since there may be multiple peripheral types on a side. Same with the chat message appearing when you connect a modem. Or you would have to add another way to see how many peripherals are there for a specific side.

commented

Not realy. Cause side is "left", "left2", "left3" ect. you just run it for each of peripheral.getNames() returns to find all peripherals and look for "left" at begining of name.

Edit: used wrong function in example. Fixed.

commented

I still think that (going with the previous example) having both a peripheral.find("rf_provider").getEnergyStored and a peripheral.find("capacitor_bank").getEnergyStored would cause more confusion to players than merging them so that the lower-priority (i.e. more generic) one will be omitted.

commented

Yea i used wrong function. I meant peripheral.getNames() to get all names and then just look for all starting with left. Then analize left side and select correct getEnergyStored depending on types?

It may be just me but makes more sense this way for me then adding ambiguity with functions that can get overwritten by other ones.

commented

Well, there would not be any ambiguity on the player side since there would only be a single method. Your suggestion would add two methods with the same name which would be ambiguous. My suggestion just needs addon devs to be careful with how they choose their priorities (i.e. actually make peripherals a higher priority the less generic they are).

commented

Hmm i can agree with your ideas. It would be interesting way to make a patches for broken peripherals if the mod-maker is not willing/able to fix it. Its just that overwriting someone else function feels wrong to me. Also letting someone else overwrite mine by setting bigger priority then mine feels bad to too. I think i need to think about this some more.

commented

As I said, it would require communication among addon devs to make sure the correct peripherals with the best methods are always the highest-priority ones for any specific block. But it would create the least ambiguity and confusion for players.

Overriding methods is something completely normal in a Java or Lua environment (you can even replace native functions in Lua), I don't see how it would be different in this case.

commented

True. You are right. And after all it is one peripheral in the end. It would make sense to wrap all its function using one wrap and have it use the best functions.

Then what i would suggest is adding a function that would allow advanced player access to those overwritten functions. That would make sure that even if the mod-maker messes up the priority system (human element tends to break often) there is a way to get correct version after all. Or if someone has a preference in versions of function.

I guess we should just ask Dan200 what he thinks of it now.

commented

I generally would like to see more opinions of other addon devs, if possible.

Calling @SquidDev and @boq in case they care.

commented

This is one of the peripheral API enhancements I'd really like to see.

This is especially useful when you have a mod like OpenPeripheral or Plethora which add methods to all inventories. When a mod provides its own peripheral, you then can't use OP/Plethora's inventory functions, which are generally more powerful.

I don't really have an opinion on merging peripherals. It seems more intuitive to only have one peripheral for each side/remote name. However, it would be nice if peripheral.find worked on all variants. (so peripheral.find("rf_provider") and peripheral.find("capacitor_bank") return the same peripheral). I guess you could implement that by having peripheral.getType return all the types.

I'm not too sure about what to do with clashing methods. If you do go the priority route (which is what Plethora and Comptronics do), I do feel there should be a way to get the masked method.

TLDR: I like the concept, I just don't know how. Then I ramble a lot.

commented

Making getType return multiple names (either as a table, or multiple return values, probably the latter for legacy compatibility reasons) sounds like a good idea.

commented

I'm not sure about the side/type_# format, to me side_# makes more sense for consistency. If you want to verify that you're getting the correct peripheral, there's peripheral.getType. I like the composite peripheral idea though.

How would composite peripherals work in the case of wired modems?

commented

Oh, are we rambling? Good, let me begin.

Why I don't like priorities

The way I see it, priorities don't solve conflics (since there is no guarantee they will be unique), but create new ones (since any addon can add (probably) peripherals with non-configurable priorities that can hide parts of other addon functionality - leaving that to community without centralized authority probably will no work).
In my opinion user should be aware of conflict, but be able to explicitly select wanted peripheral.

Also, adding priorities requires API change, which is always risky.

Selection of peripheral

To keep compatibility, I suggest reusing all existing peripheral functions, but expand side argument to something like side[/peripheraltype[_index]].
Let's say, for example, that for some block (on left side) there are three peripherals named A, B and B (from two different mods), all with same method 'f'.

Then:

peripheral.call('left', 'f') -- calls synthetic function (A+B+B).f (described in next section)
peripheral.call('left/A', 'f') -- calls peripheral A.f
peripheral.call('left/B', 'f') -- calls synthetic function (B+B).f (described in next section)
peripheral.call('left/B_1', 'f') -- calls function from one of B peripherals (left to implementation, probably registration order, since it can be partially controlled)

As bonus, peripheral.isPresent('left/A') can be used not only to check presence, but also exact type - so there is no need to align getType to new functionality.
peripheral.getNames() would return all non-conflicting peripherals (so 'left/A', 'left/B_1, 'left/B_2').

Modem-attached peripherals would remain unchanged.

To simplify selection, maybe any composite peripheral would contain extra method .as(type) that would return peripheral of more specific type - to peripheral.wrap('left/A') == peripheral.wrap('left').as('A').

Conflicting functions

I'm proposing creating composite peripherals (like 'left' in example) for compatibility, since most peripherals won't cause conflicts (inventory peripherals will have different names than energy ones). If one of conflicting methods in peripheral is called, user would get error
More that one peripheral defines this method: [list], please select more specific type or something like that. All non-conflicting methods would be called directly. Name of composite peripheral would contain names of all used peripherals.

As an exception, conflict is not present if one of the functions is defined directly on TE - then it will always be used, since it is most specific one, and external ones are ignored.

commented

Also, adding priorities requires API change, which is always risky.

Which is why I suggested adding a new interface extending IPeripheral and making others have a default priority of 0.

Your suggestion is similar to the one @dan200 wrote, and it has the same issue of being more confusing and ambiguous to the player. My suggestion would not really change any behaviour as far as players are concerned, which is one of its main benefits.

How about we actually do both? left would return a merged peripheral with priorities and all, but left1, left2 etc. would return individual ones. On the other side, peripheral.find("something") would return the merged peripheral like I suggested, but we could introduce a new peripheral.findSpecific or similar which, once again, would only return an individual peripheral instead of a merged one.

most peripherals won't cause conflicts

As I said, I already had quite a few cases of that happening. It is more common than one might think.

commented

In my opinion, merging by priority is much more dangerous - how can user tell, if method getCapacity returns EU or RF in case of conflict? That's why I propose explicit errors instead of using seemingly random value (because it would depend only on addon author) - that way player would have full knowledge what went wrong, instead of silent default action.

Anyway, main idea was to use side/type instead of side+index - that way no extra API (like peripheral.findSpecific) is needed and player will always get what it wants (i.e. either exact peripheral or nil if not present).

commented

This also makes me realize. We keep talking about side stuff but completely missing the network peripherals stuff. Whatever the choice is made it has to fit into both side peripherals and network peripherals too.

Right now i see 2 option discussed here:

  1. Merging peripheral - all peripherals provided by the block are merged into one - priority system decides what functions are used. What decides the network name for peripheral? There is some way to get masked functions. Player is provided with one unifier peripheral per block that has all functionality of said block.

  2. Multiple peripheral - all peripherals provided by the block are provided as separate peripherals - one per each - In side inventory case its "left" "left2" "left3" - in network inventory case the names of peripherals are used. Player is responsible for choosing correct one.
    Player is responsible for use of correct peripheral with correct functions form correct peripherals. There is no way to know witch ones names for networked peripherals are attached to same block.

Both approaches have ups, downs and permutations from what i see. And personally?

EDIT: Boq post 3 messages down made me rethink things. I have no preference until someone else rethinks this add add some new paint on the ideas. Cause both of them now feel wierd.

commented

@apemanzilla This was dictated by simplicity of getting exact peripheral - with side+type user can do it with peripheral.wrap('side/type'). With side+index it would require loop over peripheral.getNames and getType call for every result - of course one may use peripheral.find with filter or hide that complexity behind library function, but it doesn't seem beginners friendly.

@Wojbie about solution 1: it would be like running HTTP and SMTP on the same port. Why create merging problems where there are none? In my opinion problem here is with 'side-indexing' (for physically adjacent blocks) and not 'type-indexing' (for remote blocks), since later already works with multiple peripherals per block.

commented

Why create merging problems where there are none?

If there are no method conflicts, merging them would not have a single disadvantage. If there are conflicting methods, the priority system would ensure (provided addon devs are smart) that the most correct one is always chosen as the default (as mentioned, access to the others might be granted by other means). The suggestion assumes that addon devs can communicate, then there would be no problem like a conflicting method that may return EU or RF (in the same way that EU and RF interfaces in Java have different method names so that blocks may implement both). Assuming such a conflict existed, well, one would simply not be available but in my opinion that would still be better than presenting the player with both and confusing them by two methods with the same name that do different things.

commented

Ok, just to clarify, I'm not against composite peripherals (as a way for providing legacy functionality or initial exploration of peripherals), I'm just against choosing conflicting methods based of non-configurable (by users) priorities, instead of erroring and nudging used towards more precise interface - because for me risk of providing silently broken interface is not worth any of the (false?) simplicity achieved by hiding this problem behind automerge (it won't be easy to debug large program and finding which function suddenly returns value/1000 after modpack update, because somebody decided that 'getEnergy' has to return kilo-RF).

IMO, most of the times you want exact, consistent set of methods (RF storage, inventory, etc.) and not giant bag of random parts (without any way hint where they come from), since programs are usually operating on one aspect of peripheral. It also helps with exploring, since giant wall of text with all methods from all peripherals can be discouraging.

Secondary problem: what if you can't get all addon devs to implement optional priority interface? There will be a lot of a mess for methods with priority zero and if actual functionality depends on mod load order, it will be terribly confusing for user.

And of course, low-cost override may generate bad blood, when one addon decides to 'fix' some subset of methods form other - it's terribly easy to do something like this with priorities. I may be getting old, but I think that anything that requires cooperation from open group of people of people without any financial ties is going to crash and burn.

Anyway, I don't want to kill this thread by bikeshedding, so I will just wait for BDFL (i.e, newly crowned @dan200) decision.

commented

Well if A and B do provide the same method and trying to call it errors asking for specification... how would the player know whether A or B was the correct one in this particular instance? Maybe both A and B even have very similar names but still do slightly different things.

priority zero and if actual functionality depends on mod load order, it will be terribly confusing for user.

Behaviour would be exactly as it is currently.

commented

Well, if user gets error saying that both 'rf_storage' and 'eu_storage' override it, then he can clarify by selecting of the two interfaces - since he precisely know what he wants.

Reversing this question: if player gets one method and has no idea who provides and what it actually does, how is he going to resolve that? Or even debug this one? Remember that priorities are not going to be user visible and most of the users just download programs from pastebin.

commented

Problem is more like the original example of rf_provider vs. capacitor_bank. Some people might choose the former peripheral because it clearly is related to RF even though that one's getEnergyStored will not work for capacitor banks due to being too generic.

Both sides have ambiguity issues it seems.

commented

Some people might choose the former peripheral

That just mean they didn't read documentation and manual i think.
All programing has quirks like that.... well most.

commented

Even with this example, you think that automatic resolution (that depends on exact combination of peripheral on block and mod load order) is better than explicit selection of one of the interfaces?

If user selects one of the peripheral and decides it's not what they expected - well, they better have easy way of selecting other one - as whole set, not on per-method basis.