Integrated Dynamics

Integrated Dynamics

63M Downloads

Copying a variable card changes original's ID

pnieuwkamp opened this issue ยท 11 comments

commented

Issue type:

  • ๐Ÿ› Bug

Short description:

When you copy a variable card the new card is identical to the old card, but it mangles the old card. #725 is already describing part of it, but while the name disappearing is annoying, changing the ID has the potential to break the system.

Steps to reproduce the problem:

  1. Program a variable card
  2. Place it in a crafting grid together with a blank variable card
  3. Take the new card from the result window
  • The ID is the same as the original
  • The name is the same as the original
  1. Take the old card from the crafting window
  • The ID changed
  • The name changed
  • Neither of these are immediately obvious unless you're looking for it

Optional steps, and what makes this behavior break the system:
5. Place the old variable back in the display panel you got it from
6. Place the new variable in a different display panel where you want to view the same information
7. Go about your way
8. Decide you don't need the second screen anymore and remove it
9. Clear the new card as it's supposed to be a duplicate and you don't need it anymore
10. Oops, you just broke something that only appeared to be a duplicate but wasn't, and now everything breaks

Expected behaviour:

I expected a duplicate to be a true duplicate, not a separate card that just happens to have the same operators and the same references.
I ended up installing NBT Manipulator to change the ID of the new card to the ID of the (no longer existing) old card, as I didn't want to redo 30+ cards

Proposed fix:

I have been a programmer for a few years, but I never made any MC mods, and I don't know the ins and outs of this mod, so just looking at what I see:

  1. I can see a short term fix in keeping the ID the same and just using the first card with that ID that you find in the system. This would probably break if #357 got implemented, but nothing else sticks out yet (to me anyway).
  2. A fix that involves a lot more work (and a transition mechanism) would be to store all the variables in the variable store, and reference them from the display panels / readers / writers importer / exporter / etc. That way you don't need two (or more) 'identical' cards when you want to display something twice, or display it and use it in an exporter at the same time.
  3. Some sort of expectation management. Don't change the ID, but change the tooltip from "Clear or copy in a crafting grid" to something like "Use a crafting grid to clear the card, or make a second card with comparable settings"

Also, I think it would be more intuitive if the old one stays the same, and the new one gets the new ID / name.


Versions:

  • MC Eternal: 1.3.5.3
  • This mod: 1.1.2
  • Minecraft: 1.12.2
  • Forge: 14.23.5.2847

I've checked the change logs up to 1.1.6 but they don't mention something related. I can upgrade if you think that might help.

commented

The logic for copying variables should put the existing variable in the crafting grid's output slot, and the new variable should be left in the crafting grid.
So the variable with your original id should still exist.

Can you confirm this behavior on your end?

commented

Yeah, step 3 of the "How to reproduce" gives me an item with the original ID. That is not the issue.

The issue is that after copying the variable I do not have two copies of the same variable. I have the original variable, and I have a new, different variable that happens to have the same variableIds and operator.

Let's say I'm trying to copy a variable with the following NBT data:

{
 id:"integrateddynamics:variable",
 Count:1b,
 tag:
 {
  variableIds:
  [
   I;55,
   58
  ],
  _type:"operator",
  _id:18,
  operatorName:"operator.operators.integrateddynamics.logical.and.name"
 },
 Damage:0s
}

Currently, after copying, I have 1 item with NBT data

{
 id:"integrateddynamics:variable",
 Count:1b,
 tag:
 {
  variableIds:
  [
   I;55,
   58
  ],
  _type:"operator",
  _id:18,
  operatorName:"operator.operators.integrateddynamics.logical.and.name"
 },
 Damage:0s
}

and 1 item with NBT data

{
 id:"integrateddynamics:variable",
 Count:1b,
 tag:
 {
  variableIds:
  [
   I;55,
   58
  ],
  _type:"operator",
  _id:65,
  operatorName:"operator.operators.integrateddynamics.logical.and.name"
 },
 Damage:0s
}

where my expectation would be that I end up with two items with NBT data

{
 id:"integrateddynamics:variable",
 Count:1b,
 tag:
 {
  variableIds:
  [
   I;55,
   58
  ],
  _type:"operator",
  _id:18,
  operatorName:"operator.operators.integrateddynamics.logical.and.name"
 },
 Damage:0s
}
commented

It's not really a question; Even intentional behavior can be unwanted or unexpected.

I'm sorry, and I don't want to be a jerk, but that's the second post that has me wondering what I did wrong with the opening post, as IMHO the answer is already in there? Please tell me what part I wrote unclear so I can clarify it.

commented

Everything from your post is clear to me, and AFAICS there is no issue, as everything is working as intended.
Unless you think I am missing something?

commented

The behaviour you describe is intentional.
Every id can only be used once, that's the reason why id's exist.
These id's are used to identify a variable within a network, if multiple variables with the same id would exist, things would break horribly.

Does this clarify your question?

commented

I disagree: there is an issue. Something that is called a copy isn't a copy. This sets the wrong expectations.

It may be working as designed, but it's not working in the way the user (or at least: I) was expecting. Usually, that's not a big issue, but with the way variable cards are chained together this becomes a huge mess as it is very easy to delete the wrong variable card if you have no indication that it might be the wrong card, and there is no real way to recover from that (other than installing 3rd party tools and muck with the ID's yourself); you have to start over from the beginning (or at least, from whatever point in the chain the card was. In my case, card 18 out of 60).

The expectation is that when you call something a copy, it will be a copy. If I have two copies of something, it's safe to throw away one of them: I will still have the other one. With variable cards, throwing away one of the two 'copies' gives you a 50% chance of horribly breaking the system.

commented

How would you suggest solving this UX problem?

Creating an identical copy is not an option in any case. That's like having a URL X pointing to two different websites.

commented

As you can't edit cards (not officially anyway) the URL would be pointing to the same website. Just using the first one you find in the system would likely suffice (Option 1 I mention in the opening post), though as I don't know the internal workings if the mod this may be easier said than done.

The second option I see if fundamentally changing the way cards interact with the system. Require them all to be in a variable store, and let all the writers / importers / exporters / display panel pick from the cards in the storage (option 2 in the opening post). That way we don't need copies at all. Perhaps do away with the item altogether and use crystallized menril chunks as 'fuel' for creating variables in the store? That way you can even make checks to see if it's still being used if someone wants to delete one. Would also solve #798.

The third option I see (option 3 in the opening post) is expectation management. Change the tool tip that tells us we can copy the card and tell us that it isn't a true copy.

Come to think of it, I can think of a third option. Make variable cards pointers, references. Store the actual data somewhere else (I don't know how fast this is or how much storage is available there, but you're already keeping track of the number of cards you've written) and use the variable card as a pointer to the actual data. That way it doesn't matter how many pointers there are to the actual data, there is only one 'website' to the 'URL'.

Option 3 is by far the easiest :P Something like "Clear or copy in a crafting grid" -> "Clear or copy in a crafting grid. Warning: The two copies will have different ID's!"

commented

As you can't edit cards (not officially anyway) the URL would be pointing to the same website.

In my metaphor, variable cards represent the Web servers behind URLs. Since copying a variable card would create a new Web server, this can not be identified by the same URL.

Fundamental changes to the internals of ID as you suggest is not something I plan to do.

The tooltip change seems overkill to me, as 99% of all people will have no need for this. I could live with an addition to the book on this though. So if this something you would like to have, a pull request on this is definitely welcome.

commented

There's a book?!? :P

I did some digging (as I said, I've never made Minecraft mods) but I think I need /src/main/resources/assets/integrateddynamics/lang/en_us.lang? I'm afraid I don't speak Swedish or Simplified Chinese...

Since copying a variable card would create a new Web server, this can not be identified by the same URL.
As the person managing the load balancers at work I disagree, but I digress.

commented

I think I need /src/main/resources/assets/integrateddynamics/lang/en_us.lang?

Yep

I'm afraid I don't speak Swedish or Simplified Chinese...

Me neither :-)
The english version is the most umportant one.