hertelukas / cards

A framework and webserver to create and host card games
https://cards.lukas-hertel.de
GNU General Public License v3.0
6 stars 0 forks source link

Refactor Game-folder #63

Open ramonB1996 opened 2 years ago

ramonB1996 commented 2 years ago

Hi, I might have some time to contribute to your project. Do you look at PR's from others?

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

I would like to refactor the IGameService to be more like a base class.

Describe the solution you'd like A clear and concise description of what you want to happen.

Title and Description should be abstract properties and should always be filled for each game imo. There might be more things that can be made easier, but this was something I saw at a glance.

Describe alternatives you've considered A clear and concise description of any alternative solutions or features you've considered.

Additional context Add any other context or screenshots about the feature request here.

hertelukas commented 2 years ago

Of course. This enhancement makes a lot of sense and I will look into any PR

ramonB1996 commented 2 years ago

Hi Lukas,

I have made a diagram with my intended architecture for the Game-folder in my google-drive. I think your repository has alot of potential to add alot of games for fun, but we need to make it more extensible for the future in my opinion.

Link to class-diagram for Game-folder

Explanation:

These are some quick thoughts about my intended architecture. Please let me know your remarks and opinion, so we can discuss. I will only start with a PR, once you fully accepted my idea for it.

I would love to work on your project in the future, so hopefully we can keep working together on this, if you want to ;)

hertelukas commented 2 years ago

I looked into your UML diagram and think it makes a lot of sense to do those refactoring changes, especially making IGameService an abstract class.

Not sure about the renaming of Poker to cards, because it currently is a Poker deck and I want to be able to support different kind of cards. So "Card" might be to general, but something related to "standard 52-card deck" (https://en.wikipedia.org/wiki/Standard_52-card_deck) would be fine, wouldn't you agree? This also makes ToHtmlString() difficult to implement on the client, but if you find a clean solution, go for it.

In general, everything should be kept as general as possible, my idea of this project is to support any kind of game with any kind of card decks. I think it's important to keep that in mind for every line of code, even only small restrictions could sum up to a loss in possibilities for implementing new games.

ramonB1996 commented 2 years ago

Good points! I see we agree on the extensibility of the project.

Another question: would you allow other games (like chess, etc.) that are not a card game to be added to the library?

I am going to make a PR this week for the changes the way I see them, so you can take your time reviewing the code and making comments when needed.

hertelukas commented 2 years ago

No, if someone would try to do something like that, that would be fine, but I think we should focus on turn-based card games. At least that was my idea - coming from multiple issues while trying to play UNO online.

And thanks a lot! I will look into it as soon as I can!

ramonB1996 commented 2 years ago

Part 1 of the refactoring: PR