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

add North detection to Overworld watcher #487

Merged
merged 2 commits into from
Sep 19, 2024

Conversation

jw098
Copy link
Collaborator

@jw098 jw098 commented Sep 18, 2024

No description provided.

@@ -77,7 +88,7 @@ std::pair<double, double> DirectionDetector::locate_north(ConsoleHandle& console
if (zero_coord.first > 0.91 || zero_coord.first < 0.90 ||
zero_coord.second > 0.84 || zero_coord.second < 0.82)
{
console.log("Unable to locate the overworld radar ball, falling back on hard coded location.", COLOR_ORANGE);
std::cout << "Unable to locate the overworld radar ball, falling back on hard coded location." << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stick with console.log() for normal logging and reserve std::cout only for debugging.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we use console in locate_north, it means that OverworldWatcher can no longer use it (via detect_north), since OverworldWatcher doesn't contain console.

So, I see 2 other options:

  • Refactor OverworldWatcher to contain console in its constructor.
  • Leave console within locate_north. But then copy its code into detect_north, and remove the sections that require console. This allows OverworldWatcher to still use detect_north. But this creates code duplication.

@Mysticial Mysticial merged commit 2c6c222 into PokemonAutomation:main Sep 19, 2024
6 checks passed
@jw098 jw098 deleted the overworld branch September 30, 2024 22:18
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