davidrmiller / biosim4

Biological evolution simulator
Other
3.1k stars 435 forks source link

Allow the MOVE_LR action to go in the left. #24

Closed GregEakin closed 2 years ago

GregEakin commented 2 years ago

As written, when the level is less than zero, it goes left a negative amount. This is the same as going right with a positive amount. So, this action only goes to the right.

If the range of level is from -1.0f to 1.0f, then the code for MOVE_RIGHT is correct for this case. too For when it's less than zero, it goes right a negative amount.

But, if we limit the range of neurons between 0.0f and 1.0f, then how about setting 0.5f as moving forward, less than that is to the left, and more it to the right? The proposed fix is for this expectation.

davidrmiller commented 2 years ago

I see what you mean -- when level is negative, offset is a unit vector to the left but it gets multiplied by a negative level and becomes a movement to the right. I'm inclined to make a different change which is probably what I originally intended: If we multiply the offset by the absolute value of level, then negatives values go left and positive values go right. I.e., replace:

moveX += offset.x level; moveY += offset.y level;

with:

moveX += offset.x std::abs(level); moveY += offset.y std::abs(level);

What do you think?

GregEakin commented 2 years ago

I tried your solution first, but the block of code for MOVE_RIGHT does the same thing, without the if block and the two abs() calls:

if (isEnabled(Action::MOVE_RL)) {
    level = actionLevels[Action::MOVE_RL];
    offset = indiv.lastMoveDir.rotate90DegCW().asNormalizedCoord();
    moveX += offset.x * level;
    moveY += offset.y * level;
}

Is it okay that this action level goes from -1.0f and 1.0f, where the other action levels are between 0.0f and 1.0f?

davidrmiller commented 2 years ago

I believe the MOVE_RL action level has a symmetry around zero. Your last solution above is much more elegant than my if-tests and abs() calls. I would gladly accept a pull request with that code.

GregEakin commented 2 years ago

Thank you.