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

Crash from card with type mismatch #1301

Closed
met4000 opened this issue Aug 19, 2023 · 4 comments
Closed

Crash from card with type mismatch #1301

met4000 opened this issue Aug 19, 2023 · 4 comments

Comments

@met4000
Copy link
Contributor

met4000 commented Aug 19, 2023

Issue type:

  • 🐛 Bug

Short description:

t3 from below causes a crash when name(t3) is placed in a display panel, or if a card is placed in the bottom slot of a materialiser when t3 is in the top slot (placing t3 in the top slot alone is fine).

Steps to reproduce the problem:

t1 = append([], arithmeticIncrement)
t2 = append(t1, 1.1d)
t3 = reduce1(apply, t2)

Note that t4 = add(t3, 7i) in a display panel yields 9.1d as expected (i.e. it correctly detects that the two types are Double and Integer, and outputs Double), however name(t4) also causes a crash.
negation(t3) results in the error 'Attempted to cast Double to Operator, for value "2.1"', but does not cause a crash (I'm not sure what the behaviour would otherwise be, so I guess this is as expected).
The LP does not allow negation(t4) to be created, giving the error 'The operation negation received an input with type Number at position 1 while the type Operator was expected.'.

From BlockEntityMaterializer.java#L113 (final IValue value = variable.getType().materialize(variable.getValue());) it looks like there might be a problem with the getType of the variable returning one thing (i.e. Operator), but the actual getValue of the variable being a different type (e.g. Double).

Expected behaviour:

Either being consistent with increment(t3) at least partially working, i.e.:
Materialising t3 yields a card with the correct type and value (i.e. a Double card with value 2.1).
name(t3) yields the string "2.1" (i.e. the same value as name(2.1d)), and similar behaviour for name(t4).

Or, any attempt to evaluate t3 results in an error due to the incorrect typing:
Any of placing t3 in a display panel, placing increment(t3) in a display panel, or trying to materialise t3 yields an error like 'Attempted to cast Double to Operator, for value "2.1"' (the error displayed currently when trying to evaluate negation(t3)).


Versions:

  • Minecraft: 1.20.1
  • Forge: 47.1.0

261787947-66abc967-25a5-4584-aa37-1e28f2dd1db7

Log file:

(note: I have had a lot of crashes recently, so I'm fairly sure these are the correct crashes and that they are labelled correctly, but if something doesn't seem right I may have made a mistake)

https://pastebin.com/asTsNCBb - materialiser crash
https://pastebin.com/hzNsr9Yj - name(t3) crash
https://pastebin.com/GAxqSMiZ - name(t4) crash

@rubensworks
Copy link
Member

Thanks for reporting!

@met4000
Copy link
Contributor Author

met4000 commented Aug 19, 2023

If other people have a similar issue: you can get around the type checking by doing the reduce on an Any array, rather than the elements directly e.g.:

n1 = append([], arithmeticIncrement)
n2 = append(t1, 1.1d) -- the list we want to `apply` over

reduce_op(r, v) = append(r, apply(getLastListElement(r), v))

n3 = reduce(reduce_op, n2, [id])
n4 = getLastListElement(n3)

increment(n4), name(n4), and materialising n4 all work correctly.

@met4000
Copy link
Contributor Author

met4000 commented Sep 3, 2023

I don't know if this should be a new issue or still part of this issue, but there are operators other than those previously mentioned here still causes a crash. uname is the only one I have found - I thought that isNull, isNotNull, and relationalEquals might also crash, but they were fine at least with how I tested them.

Even if operators that cause a crash like this should be considered and fixed individually (such as with the commit that fixed name), would it be possible to catch these errors globally in some way to prevent crashes from operators that might have been missed (particularly, any operators that might be added in future)?
E.g., even if the 'correct' fix for uname is similar to 2ea2d60, is it possible to have something that would prevent the game from crashing even if this fix wasn't added?

Stacktrace:
uname crash (evaluating uname(reduce1(apply, append(append(append([], stringConcat), "a"), "b")))):

java.lang.ClassCastException: class org.cyclops.integrateddynamics.core.evaluate.variable.ValueTypeCategoryAny cannot be cast to class org.cyclops.integrateddynamics.api.evaluate.variable.IValueTypeUniquelyNamed (org.cyclops.integrateddynamics.core.evaluate.variable.ValueTypeCategoryAny and org.cyclops.integrateddynamics.api.evaluate.variable.IValueTypeUniquelyNamed are in module [email protected] of loader 'TRANSFORMER' @7f7b6639)
at org.cyclops.integrateddynamics.core.evaluate.variable.ValueTypeCategoryUniquelyNamed.getUniqueName(ValueTypeCategoryUniquelyNamed.java:22) ~[IntegratedDynamics-1.20.1-1.18.0.jar%23167!/:1.18.0] {re:classloading}
at org.cyclops.integrateddynamics.core.evaluate.operator.Operators.lambda$static$44(Operators.java:811) ~[IntegratedDynamics-1.20.1-1.18.0.jar%23167!/:1.18.0] {re:classloading}
at org.cyclops.integrateddynamics.core.evaluate.operator.OperatorBase.evaluate(OperatorBase.java:152) ~[IntegratedDynamics-1.20.1-1.18.0.jar%23167!/:1.18.0] {re:classloading}
at org.cyclops.integrateddynamics.core.evaluate.expression.LazyExpression.evaluate(LazyExpression.java:43) ~[IntegratedDynamics-1.20.1-1.18.0.jar%23167!/:1.18.0] {re:classloading}
at org.cyclops.integrateddynamics.core.evaluate.expression.LazyExpression.getValue(LazyExpression.java:65) ~[IntegratedDynamics-1.20.1-1.18.0.jar%23167!/:1.18.0] {re:classloading}
at org.cyclops.integrateddynamics.core.part.panel.PartTypePanelVariableDriven.update(PartTypePanelVariableDriven.java:129) ~[IntegratedDynamics-1.20.1-1.18.0.jar%23167!/:1.18.0] {re:classloading}
[... stacktrace continues ...]

(tested in 1.20.1-1.18.0)

@rubensworks
Copy link
Member

would it be possible to catch these errors globally in some way to prevent crashes from operators that might have been missed (particularly, any operators that might be added in future)?

Not easily I'm afraid. Unless we'd catch all ClassCastExceptions or RuntimeExceptions, but then we might be catching actual programming errors, which will open another whole can of worms.

Let's just fix the uname in a similar manner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

2 participants