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

Restructure the project layout and improving package usage #71

Merged
merged 17 commits into from
Feb 19, 2024

Conversation

Raining-Cloud
Copy link
Collaborator

@Raining-Cloud Raining-Cloud commented Feb 2, 2024

Description

This PR resolves #62, due to this is a major change in the way the end-user interacts with our product it was not really possible to split this into smaller PR's.

The new structure of our project looks like this:

comprl
|   tree.txt
|   
+---client
|       agent.py
|       interfaces.py
|       networking.py
|       __init__.py
|       
+---server
|   |   app.py
|   |   interfaces.py
|   |   managers.py
|   |   networking.py
|   |   util.py
|   |   __init__.py
|   |   
|   \---data
|           interfaces.py
|           sql_backend.py
|           __init__.py
|           
\---shared
        commands.py
        types.py
        __init__.py

I restructured the project so that we have a module (in python this is a .py file) for every component of the packages (which are folders in python). Generally we can say networking modules contain all twisted related code, interfaces contain interfaces needed for the abstraction between networking and logic, like managers.

The networking also got completely separated from the rest of our code, for this I used interfaces which get passed to the corresponding twisted-networking implementation. Both the server and the client networking modules provide a global function which is capable of connecting a subclass from the given interface to the networking code. By this we don't need to worry about networking in the rest of our logic, also we don't even know what drives our code. So we could add tests for the non networking parts.

A major change also included redesigning the entry point of our server. We now have the app module in the server package which implements not only our server class, but also includes the main function. Previously the end user needed to start the server by creating a server object in custom python code, this got completely scrapped. The server now can be started via the command line with additional args or by providing a toml config file. The toml file causes the upgrade to python 3.11 due to the 'tomllib' gets introduced with it.

Update: I added support for python 3.10 as @luator suggested

(If you are wondering where the examples went, I moved the outside our package. You can find them in /examples/ now.)

@Raining-Cloud Raining-Cloud added documentation Improvements or additions to documentation enhancement New feature or request server labels Feb 2, 2024
@Raining-Cloud Raining-Cloud self-assigned this Feb 2, 2024
@Raining-Cloud Raining-Cloud marked this pull request as ready for review February 16, 2024 01:06
Copy link
Collaborator

@Cari1111 Cari1111 left a comment

Choose a reason for hiding this comment

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

Thanks for the restructuring! Everything is a lot clearer now. The new structure of the code makes sense and helps to reduce the number of files.

I tested the code locally with the simple game and it still works. There are only a few suggestions:

  1. It would help a lot if you would update the README file on how to execute the project. It took a little bit to figure that out.

  2. The hockeygame and the rock_paper_scissors game don't work anymore. It would be best to make a new issue to update them.

  3. other changes below.

comprl/client/interfaces.py Show resolved Hide resolved
comprl/server/data/sql_backend.py Outdated Show resolved Hide resolved
comprl/server/interfaces.py Outdated Show resolved Hide resolved
comprl/server/managers.py Outdated Show resolved Hide resolved
comprl/server/networking.py Outdated Show resolved Hide resolved
comprl/server/networking.py Outdated Show resolved Hide resolved
examples/simple/game.py Outdated Show resolved Hide resolved
comprl/server/__main__.py Show resolved Hide resolved
@Raining-Cloud
Copy link
Collaborator Author

Raining-Cloud commented Feb 17, 2024

Thanks for the restructuring! Everything is a lot clearer now. The new structure of the code makes sense and helps to reduce the number of files.

I tested the code locally with the simple game and it still works. There are only a few suggestions:

1. It would help a lot if you would update the `README` file on how to execute the project. It took a little bit to figure that out.

2. The `hockeygame` and the `rock_paper_scissors` game don't work anymore. It would be best to make a new issue to update them.

3. other changes below.

Yeah I forgot to mention how exactly to start the server, thought I already added it last week
You can start the server with python -m comprl.server --config="path/to/config.toml", the agents can be started like scripts

Copy link
Collaborator

@Cari1111 Cari1111 left a comment

Choose a reason for hiding this comment

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

All requested changes are impementet and the code still works.

Copy link
Collaborator

@Cari1111 Cari1111 left a comment

Choose a reason for hiding this comment

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

I missed that the end method in IGame still uses dummy values.

comprl/server/interfaces.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@Cari1111 Cari1111 left a comment

Choose a reason for hiding this comment

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

Now are all requested changes implemented and the code works

@Raining-Cloud Raining-Cloud merged commit f42a268 into main Feb 19, 2024
4 checks passed
@Raining-Cloud Raining-Cloud deleted the raining-cloud/restructure branch March 4, 2024 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request server
Projects
Development

Successfully merging this pull request may close these issues.

restructure current project layout
2 participants