XaverStiensmeier / ilarisdiscordbot

A discord bot for the ilaris ttrpg
GNU General Public License v3.0
0 stars 0 forks source link

Create a group class #75

Closed lukruh closed 1 month ago

lukruh commented 5 months ago

There are some further adjustment I'd suggest to the structure of groups:

From a technical perspective I think the organize_group module should be rewritten a little bit, the functions might be more usefull as a method in a class. I think many parts of the code repeat the same patterns (i.e. reading, sanitzing guild and group name, user id and more to pass it to many calls to all the organize_group funcions). It could become much 'DRY'er if we use a class Group instead with a set of methods, that can be applied to a single group. Getting or creating, editing and saving this group from anywhere should be as simple as:

my_group = Group(ctx, "Name of my Group")
my_group.description = "New description Text"
my_group.save()

guilds, sanitizing permission checks and more could all be done within the class based on name and context (or interaction). Beside making it easier to use this provides another layer of abstraction for our data. We could for example switch to a database or another file format or sync with an api with only changing the read/save functions in the class. I'd go the extra mile in the data class to keep all data in properties and map them to a dictionary (rather than diretly store it in a dictionary) because this allows a bit more flexibility if we want to reference discord obejcts in the class, but only read/write the corresponding ids to the yml file. I think this would provide a lot of cool stuff like

member_names = [player.name for player in my_group.players]

This would be very nice to generate messages/views with buttons (view) very generic..

ctx.send(my_group.as_message(), view=my_group.buttons())
lukruh commented 5 months ago

organize_group could roughly be changed to a Group class following this changes (most of the function body would stay the same just without the args):

# keep on module level:
list_groups(guild, show_full=False)

# class functions:
create_group()

# methods/properties:
is_owner(guild, group, author_id) -> group.is_owner(author)
group_exists(guild, group) -> group.exists()
destroy_group(guild, group, author=None) -> group.destroy()
get_main_channel(guild, group) -> group.main_channel
set_key(guild, group, key, value) -> group.key or group.data
remove_player(guild, group, player) -> group.remove_player(player)
add_self(guild, group, player) -> group.add_player(player)
remove_self(guild, group, player) -> group.remove_player(player)

not sure about check/add/remove channels, this could be part of group.py or a separate module for server stuff.

lukruh commented 5 months ago

updated, list in first post..

lukruh commented 5 months ago

I also came across TinyDB https://github.com/msiemens/tinydb#example-code. Right know I think reading/saving a yaml file is fine, there are not much operations, but if we ever think about using something db like to use more efficient queries etc.. this might be worth a try (as an alternative to sqlite etc..).

XaverStiensmeier commented 5 months ago

yea I've heard of TinyDB. Tbh. most times when I use a db for a small project I use sqlite though. I am fine with both but don't feel like it is currently necessary.

lukruh commented 5 months ago

Some random updates that I made during creating the class:

lukruh commented 5 months ago

I havent tested everything and some TODOs are still in the code, but I opened the PR to start some discussion about the code. There is a lot comming, don't want to get this PR too big. The organize_gorups.py is completly rewritten as a class (roughly same amount of code but some additional functionality) and using it saves a lot of code in the actual commands (groupsCog has less code, even though I added commands, buttons and documentation). I'm not 100% happy with some solutions, but I think it goes in the right direction ;).