devilutionX icon indicating copy to clipboard operation
devilutionX copied to clipboard

:recycle: Refactor RandomWalk

Open canassa opened this issue 3 years ago • 4 comments

:sparkles: ~~Adds the Shuffle helper function~~ :sparkles: Adds WalkAny :recycle: Changes RandomWalk to receive a monster reference (closes #4905) :memo: Minor documentation fixes :fire: Merges RandomWalk2 into RandomWalk

canassa avatar Jul 12 '22 11:07 canassa

After submitting this PR I came up with this version that doesn't require the Shuffle:

bool RandomWalkNoShuffle(Monster &monster, Direction md, bool narrow = false)
{
	// Tries to walk in the direction md.
	if (Walk(monster, md))
		return true;

	bool ok = false;
	// Tries the next adjacent directions.
	if (FlipCoin()) {
		ok = WalkAny(monster, { Left(md), Right(md) });
	} else {
		ok = WalkAny(monster, { Right(md), Left(md) });
	}

	// Tries the next adjacent directions.
	if (!ok && !narrow) {
		if (FlipCoin()) {
			ok = WalkAny(monster, { Left(Left(md)), Right(Right(md)) });
		} else {
			ok = WalkAny(monster, { Right(Right(md)), Left(Left(md)) });
		}
	}

	return ok;
}

It's only slightly longer than the Shuffle version and it is still more readable than the original (I think). What do you guys think?

canassa avatar Jul 12 '22 11:07 canassa

Both uses of shuffle in this pr are on a list of two choices, it's not enough of an advantage to make up for the readability issues so I think your proposed version in the comment above is better.

ephphatha avatar Jul 13 '22 08:07 ephphatha

Both uses of shuffle in this pr are on a list of two choices, it's not enough of an advantage to make up for the readability issues so I think your proposed version in the comment above is better.

I totally agree with you, unless the Shuffle could be used in other parts of the codebase I don't think this justifies adding it. I will update the MR with the second version

canassa avatar Jul 13 '22 09:07 canassa

Alright, the PR is updated to the version without Shuffle

canassa avatar Jul 13 '22 09:07 canassa