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

AOT safe #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

AOT safe #2

wants to merge 1 commit into from

Conversation

uurha
Copy link

@uurha uurha commented Oct 10, 2020

No description provided.

@mcatanzariti
Copy link
Member

Sorry I cannot accept this pull request:

  1. Tests are failing
  2. It is not just about AOT issues: you add different mathematical operators.
  3. There is a backward compatibility break on operator ^

Please, just fix the AOT problem. I won't accept additional operators. You can always add new mathematical functions by using the RegisterFunction method.

Thank you,

Michael

@uurha
Copy link
Author

uurha commented Oct 10, 2020

Sorry I cannot accept this pull request:

  1. Tests are failing
  2. It is not just about AOT issues: you add different mathematical operators.
  3. There is a backward compatibility break on operator ^

Please, just fix the AOT problem. I won't accept additional operators. You can always add new mathematical functions by using the RegisterFunction method.

Thank you,

Michael

  1. Tests failed due to Unable to cast object of type 'Dahomey.ExpressionEvaluator.StringLiteralExpression' to type 'Dahomey.ExpressionEvaluator.INumericExpression'. in file "src/Dahomey.ExpressionEvaluator.Tests". It wasn't changed by me.
  2. Issue was about math function not working in AOT. And this wasn't specified in description.
  3. Operator ^ was replaced with operator ^^.

@mcatanzariti
Copy link
Member

  1. Tests are ok on the main branch. But I will double check
  2. I will not accept to add math functions in the library. Did you test AOT with RegisterFunction
  3. I will not accept the change from ^ to ^^ because it is breaking change

@uurha
Copy link
Author

uurha commented Oct 10, 2020

2. Did you test AOT with RegisterFunction

  1. Yes I test in Unity 2019.4.1f it's not wotking
  2. But now all math and logic methods working. (I think it's easier to replace ^ for power with ^^)

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

Successfully merging this pull request may close these issues.

2 participants