kreicer / dice-roller-bot

Discord bot for rolling dice.
GNU General Public License v3.0
15 stars 3 forks source link

Adding COD and WOD dice system #16

Closed novahorizon closed 8 months ago

novahorizon commented 10 months ago

CoD (Chronicles of Darkness) and WoD (World of Darkness) both use a d10 system and sums the number of dice that roll above a certain value.

CoD counts the number of dice that equal or exceed 8, and always explode on 10, but may explode on 8 or 9 as well.

WoD counts the number of dice that equal or exceed a difficulty rating provided by the GM, always explode on 10, and fail (subtract) on 1.

The systems can be used by issuing the roll command with the dice pool, "cod" or "wod" as the delimitator, and the explode value/difficulty rating respectively.

For example:

CoD: Dice pool of 7d10 with "9-again" would be /roll 7cod9

WoD: Dice pool of 7d10 with difficulty 6 would be /roll 7wod6

Reworked dice system into a couple classes to hopefully make future changes more intuitive.

added [highlighting] to certain dice results

kreicer commented 10 months ago

Hello, mate.

First of all, I wanna say thank you for the interest to this bot. This is really matter for me and very inspired.

I just return from vacation and didn't had time to explore your changes deeply yet. I have a question: 7cod9 and 7wod6 will be the same as 7d10/exp:9/suc:8 and 7d10/exp/suc:6/fail if we will add 2 new postfixes and mechanic to use more than 1 postfix per dice (was planned in 2.1.0)?

I asking cos I would prefer keep the main functional of the bot clear from game systems (fate is exception but on first implementation I tried make it as postfix also, current implementation - hack).

I see around 3 ways: 1) Add new postfixes "success" and "fail". And mechanic to use multiple postfixes.

2) Add new game system specific postfixes. Like cod and wod. Example: 7d10/cod:9 and 7d10/wod:6

3) Move code related to cod and wod systems to separate cog. This allow make all even simple with CMDs like "/cod 7 9" and "/wod 7 6" or maybe "/darkness 7cod9".

What do you think about it?

novahorizon commented 10 months ago

Ah. I understand the desire to keep the system clear. I didn't understand the cog system well enough to try and implement something like that myself.

As part of this PR completely rewrites the rolling mechanic, I believe option 1 would be the least compatible solution.

Option 2 could be something that simply becomes an alias in the check_dice_dict() method and easy to rewrite for. (This was not the case in the initial PR, but I've updated it quite a lot during your vacation. I hope you went somewhere nice and had a good time.)

Option 3 I imagine is also quite compatible. I would probably go with the /darkness command personally to reduce command clutter.

--

That said, now that the PR includes the rewrite for the "DiceBucket" class and "Dice" class, I think Fate could be rewritten into a postfix to generate the new type as well.

I wrote those two classes to hopefully make it easier to implement new postfixes and new game systems. I believe turning postfix into a list of tuples ("postfix", value) would be doable within the DiceBucket class with little effort.

Edit: Actually, I'll switch Cod, Wod, and Fate to all be postfixes, and then update the PR.

Edit 2: Ok, maybe it was a little more work to switch those around than I expected.. But it's in there now.

kreicer commented 9 months ago

Hey! Sorry for my disappear - I just had duty to release 2.1 as soon as possible. Now I have time to integrate your changes into main code, if you still interested.

novahorizon commented 9 months ago

I am still interested, yes. There have been many changes in your code however, so it may take a while for me to resolve any conflicts between our branches. There are some fundamental differences in our branches at this point as well. For us to merge, I would like your advice on how we should proceed.

kreicer commented 9 months ago

Sure. I will look on conflicts asap and write my thinks.

kreicer commented 9 months ago

Hey. I did looked at code.

Few questions:

Right now we surely can integrate cod and wod postfixes to current schema. About classes - I bad at OOP. So, I should run stable version of your code changes to understand how it works - will do it this weekend.

kreicer commented 8 months ago

I'm done with code inspection. I really like idea with class for dice and some other ideas. I just cant resolve code conflicts between our versions. Too many changes.

I think we can add changes piece by piece. Lets keep for this MR only new postfixes: its model, checks and add it to current roll flow.

kreicer commented 8 months ago

Hey, mate. Will you add changes into code?