feat: multi-expression implemented#19
feat: multi-expression implemented#19guptanshuman124 wants to merge 2 commits intoOPCODE-Open-Spring-Fest:mainfrom
Conversation
|
Also, implicit functions, polar functions and parametric relations do not work anymore (you have simply removed all the required code for those features). Is this intentional? |
|
@nsh07 I accidently removed the features of implicit function polar function and parametric relation. I would do a commit by tomorrow evening as I was doing some refactoring in code to remove the clutterinside main application.cpp file. |
|
Sure, take your time. Your implementation is really good, and it is exactly what I was looking for. |
| namespace App::Core { | ||
|
|
||
| struct Expression { | ||
| std::array<char, 1024> expr; // expression as char array |
There was a problem hiding this comment.
could this instead be a std::string?
There was a problem hiding this comment.
I was trying this but faced some issues back then. As I remember that the exprtk compile and expression function require array of character. Not sure about it.
I would try to reuse it and check it out.
There was a problem hiding this comment.
That is not correct. std::string is the primary input parameter for the string representation of an expression for use with the compile method.
https://www.partow.net/programming/exprtk/exprtk.hpp_.html#line_24640
There was a problem hiding this comment.
Ok got it, will change the implementation. There might be something else back when I was structuring it. Thanks for the help
| for (size_t i = 0; i < functions.size(); ++i) { | ||
| if (!functions[i].visible) continue; | ||
| expressions[i].register_symbol_table(symbolTable); | ||
| parser.compile(functions[i].expr.data(), expressions[i]); |
There was a problem hiding this comment.
Make sure to check the return value of the compile method. if it is false the function should not be further evaluated.
| for (size_t i = 0; i < functions.size(); ++i) { | ||
| if (!functions[i].visible) continue; | ||
| expressions[i].register_symbol_table(symbolTable); | ||
| parser.compile(functions[i].expr.data(), expressions[i]); |
There was a problem hiding this comment.
Functions should only be compiled when they change. The Expression struct should perhaps hold the exprtk::expression instance. As constructing/destructing exprtk::parser/symbol_table/expression types on a per frame basis will be very expensive.
|
Thank you so much for your review @ArashPartow @guptanshuman124 please also resolve these change requests while you're working on the PR |
@ArashPartow thank you for the valuable feedback.I will take care of it. @nsh07 my current progress
|
3 participants
Description
Semver Changes
Issues
Checklist