Add perturbation parameters and status display #289
Conversation
Start with current master and build with updated UI files from last attempt Will try to rebase and see if we are up to date
|
Not quite there yet... Getting closer... Ubuntu working yay!. NJow it's time for bed. |
|
you can run the tests locally with ctest. Assuming you used to run the tests. |
LegalizeAdulthood
changed the title
There was a problem hiding this comment.
These changes are easily unit-testable, so now is a good time to learn about unit testing the changes you're making. There are existing tests for validating parameter handling and validating make_batch_file output (e.g. calls to put_param).
The test file directory structure mimics the directory structure of the code that is being tested.
libid/engine/calcfrac.cpp
Outdated
| break; | ||
| } | ||
| goto x_symmetry; | ||
| goto xsym; |
There was a problem hiding this comment.
Don't undo the label renames.
libid/ui/cmdfiles.cpp
Outdated
| if ((int)std::strlen(value) > ID_FILE_MAX_PATH || validate_luts(g_map_name.c_str())) | ||
| { | ||
| goto bad_color; | ||
| goto badcolor; |
There was a problem hiding this comment.
don't undo the renaming of the labels
| return CmdArgFlags::FRACTAL_PARAM | CmdArgFlags::PARAM_3D; | ||
| } | ||
|
|
||
| // tolerance=? |
There was a problem hiding this comment.
don't introduce a new parameter for tolerance; it's an extra value for the perturbation parameter, e.g. perturbation=auto/1e-4
| } | ||
|
|
||
| // perturbation=? | ||
| static CmdArgFlags cmd_perturbation(const Command &cmd) |
There was a problem hiding this comment.
existing parameters let you specify short unique prefixes for the values, e.g. y, n or a; see the other parameter handlers in this file for examples.
There was a problem hiding this comment.
This is only implementing the "happy path" and needs unit tests. In addition to recognizing the single letter shorthand it needs to reject bad values.
| std::string p = cmd.value; | ||
| if (p == "auto") | ||
| { | ||
| g_perturbation = PerturbationMode::AUTO; |
There was a problem hiding this comment.
Instead of adding another global variable, add setter/getter functions and encapsulate the state as a static variable or member of the PertEngine class.
| } | ||
| } | ||
|
|
||
| g_use_perturbation = is_perturbation(); |
There was a problem hiding this comment.
Don't add another global variable, create getter/setter functions instead.
libid/ui/make_batch_file.cpp
Outdated
| if (colors_only) | ||
| { | ||
| goto do_colors; | ||
| goto docolors; |
There was a problem hiding this comment.
here and elsewhere, don't rename labels
|
|
||
| if (g_perturbation != PerturbationMode::AUTO) | ||
| { | ||
| put_param(" %s=%s", "perturbation", (g_perturbation == PerturbationMode::YES) ? "yes" : "no"); |
There was a problem hiding this comment.
Don't use %s for a string literal, just write the text as part of the format string, e.g. put_param(" perturbation=%s", ...
Needs to handle optional tolerance here
|
I've been a little busy the last couple weeks, first getting my Utah C++ Programmers presentation ready and then dealing with (ugh) taxes and a trip to Vegas this week. I should be able to look at it later this week when I'm in Vegas. |
|
I also have been preoccupied with Anne's funeral and my health issues. I will be pleased to have these changes integrated into master as it is a pain to do it every time we try to merge my changes. |
|
The first step in getting these pull requests to green builds is to get them rebased onto master. |
|
There have been so many commits since these pull requests were made that it is probably best to close these ones and start again from pulling down the current master and adding changes from there. Then we can create a new pull request and rebase from there. |
2 participants
Start with current master and build with updated UI files from last attempt Will try to rebase and see if we are up to date