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

Proposal: Stepper Refactor #717

Closed
elicwhite opened this issue Jul 24, 2016 · 11 comments
Closed

Proposal: Stepper Refactor #717

elicwhite opened this issue Jul 24, 2016 · 11 comments

Comments

@elicwhite
Copy link
Contributor

We have almost refactored everything we need to start implementing the behavior tree proposal. The biggest hurdle we have left (that I know of) is the current stepper.

Right now the stepper keeps track of the spiral it needs to walk, and moves towards the center of each part the spiral, and then calls work_on_cell for each cell. In order for it to be compatible with an event loop style system (and thus the behavior trees), is for it to take a single step in the correct direction and update some piece of memory with the current cell, and then have work_on_cell run ​_after_​ the stepper and process that cell.

Essentially what that would require is for the stepper to not walk ​_all_​ the way to its goal but to instead take a certain number of steps towards that goal and then return control to the rest of the program.

More concretely, I propose we split the current stepper into two parts:

  1. Utility class walk_towards
  2. Spiral Walker

The walk_towards class should keep track of where it was initialized, where it's end goal is and take steps in that direction every time a function like take_step is called. This same api would be able to be implemented with the current walk_towards, as well as a more complicated approach like the Polyline one in #697

The spiral walker would keep track of where it's center point is, and on every tick, do nothing if walk_towards hasn't finished, and if it has, call walk_towards to the next point. This would enable us to have different walkers. Such as one that walked to specified hotspots, or walked from LA to NY and back.

Any thoughts?

@solderzzc
Copy link
Contributor

solderzzc commented Jul 24, 2016

The walker is hardly extensible. I agree the refactor. We may need more input or start ?

@reddivision
Copy link
Contributor

just to be clear, Spiral Walker is what's actually controlling the walk patterns/logic, in this case? walk_towards is just a dummy "I'm here, I'm told to go there, I go there" class?

@reddivision
Copy link
Contributor

I'm not well-versed enough in python to know off the top of my head, but would it be a good idea/possible to implement a factory pattern to enable custom class creation based off an inheritance heirarchy that has the "walk_towards" functionality baked in? then someone could config their bot to load the default walker, or someone could create a new walker class and config the bot to load that?

@elicwhite
Copy link
Contributor Author

@reddivision That is correct.

The biggest change of this proposal though isn't the splitting apart of those two things. The biggest change is that the stepper won't walk all the way to the end goal on each call. It will be called more frequently and be expected to take a few steps in the correct direction before returning control to the rest of the program.

Right now, the bot essentially says this:

while true: #right now this loop ticks every 10 minutes or so as a spiral is finished
  walk a spiral calling clear_inventory, evolve_pokemon, spin_forts, catch_pokemon on each step

and we will be turning it into this:

while true: # now this loop will tick every few seconds
  take a few steps in the intended direction along the spiral
  clear the inventory
  evolve pokemon
  spin forts
  catch pokemon

@elicwhite
Copy link
Contributor Author

@reddivision The approach we will be taking to let people configure different types of walkers they want to use is outlined in the behavior tree proposal: #142

@MikeDX
Copy link
Contributor

MikeDX commented Jul 24, 2016

Can I suggest we use the tsp package for this https://pypi.python.org/pypi/tsp

From our starting location (wherever the bot starts) we should search the map around us (based on max steps) to find all the pokestops with the distance limit

then visit each one based on the solve() method of tsp

@elicwhite
Copy link
Contributor Author

elicwhite commented Jul 24, 2016

@MikeDX I think something like tsp is good, but out of scope for this refactor. This refactor would enable us to replace the spiral walker with something that uses tsp and would have no impact on any other parts.

The ideal outcome of this refactor is to have no behavior change, but set ourselves up for better behavior modifications after.

@MikeDX
Copy link
Contributor

MikeDX commented Jul 24, 2016

Sorry, I had assumed foolishly of course that the walk_towards was already independent and could read the 'next stop' from the tsp.solve() data.

I'll let you guys handle this refactor, whilst I go see if i can fix this blasted login expiry bug :)

@reddivision
Copy link
Contributor

Reading through the items in the other proposal, but, ultimately, I believe we're aiming for the same thing, so +1 from me.

@MEGAMERICAN
Copy link
Contributor

One thing I would love to see is the ability to import a list of GPS coordinates to be used as waypoints and I see you kind of have that proposed in #142. I live in a city with a lot of lakes that have paths around them and the current walking system is terrible near water.

@elicwhite
Copy link
Contributor Author

@MEGAMERICAN That's the plan. Instead of a SpiralWalker we can have a WaypointWalker or a DestinationWalker or a WanderWalker. The code for that will be able to be completely separate from everything else.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants