Minimum Product: moved managers to their own package

[danalex97]

Fixes #257

[OlafSzmidt]

Ah... It's the os.path I missed. Good job:)

[danalex97]

Also changed it to work on Windows just now. It was using /../...

[OlafSzmidt]

Does this now work on both UNIX and Windows?

[danalex97]

This particular class, I think so.

[OlafSzmidt]

Just pulled from your fork. Works on UNIX (but more and more often I'm starting to see the socket error with an address being in use now and I need to kill processes manually. That's a different story though and could be because I stop tests mid-way sometimes).

Can't approve as I'm not requested.

[OlafSzmidt]

Would it be possible to replace the '..', '..', '..' with an absolute path?

[CelineBoudier]

We need to be cross platform, anything with / inside won't work on Windows

[danalex97]

@CelineBoudier Removed the remaining "/". Good catch 👍
@OlafSzmidt There are some solutions here, but I don't find those nicer. 😞

Regarding the socket error, that is probably due to the tests being stopped halfway. We could catch the signal and kill those automatically, but idk if is worthed spending time on that. You can do something like pkill -9 python to kill by hand if you work on only one Python project at a time.

[ramonfmir]

pkill -9 python... classic @danalex97

[CelineBoudier]

might make more sense to import WaitDummy and DummyAvatarManager then the Directions one separately?

[danalex97]

👍

[OlafSzmidt]

Squash?

[danalex97]

👍

[OlafSzmidt]

Can't approve. @CelineBoudier

[danalex97]

Apparently, one can re-request reviews? 🤔

[OlafSzmidt]

Github isn't smart enough to notice that a rebase is actually a change, so you need to dismiss old review and request a new one, yep :P

[CelineBoudier]

I'll approve when it's more squashed :)

[CelineBoudier]

So actually, I think that:

• We should maybe think about keeping workers with workers and turns with turns rather than creating a manager folder?
• If a WorkerManager is actually the Worker service, while not call it Worker without the manager? If it does something specific, we should name it so we understand what is actually managed if it makes sense?

[danalex97]

@CelineBoudier I don't find this harmful nor beneficial, so I guess we could leave it like it is. Regarding the naming I think the name WorkerManager is quite good, as its only purpose is actually keeping an updated set of workers that can be pinged for actions by the TurnManager.

Should we close this, then?

Another comment not necessarily on this topic, but on refactoring: If the purpose of restructuring the folder structure is to group the dependencies together, I think the main concern should be breaking the world_map class in smaller components and removing unnecessary dependencies. (as I think that is the hardest class to work with and will probably change a lot as new features appear)

[OlafSzmidt]

#282.