Skip to content

Conversation

saif-ellafi
Copy link

Dear Steve,

I am not sure whether you'll like or accept this PR, but I wanted to share it anyways.
This proposal allows for an external application to use a custom dice roller which handles its own dice rolling functionality, including that of asynchronous calls.

I find this fantastic for allowing the dice roller to interact with the external world and customize randomness, among many other things.

Caveat: it breaks all tests :D but it works fine. I guess the Futures change the order of all rolls. Even when I added a simultaneousRolls parameter that allows for queuing up these calls.

Feedback welcome

@stevesea
Copy link
Collaborator

stevesea commented Jul 5, 2025

I find this fantastic for allowing the dice roller to interact with the external world and customize randomness, among many other things.

I am not following you. Currently, the client application can supply its own RNG; and it can register for callbacks or traverse the result data to render the die rolls or otherwise react to the outcome of the rolls.

Can you clarify what you mean by 'interact with the external world'?
What additional use cases are allowed by the library calling back out to the client application to perform a dice roll?

In your other PR, you'd mentioned bluetooth dice. I'm not familiar with how those work and what the API is like.

What would that interaction with the user look like?
Here's what I'm picturing, is it close to what you're thinking?

  1. user connects dice via bluetooth to app
  2. user enters a dice expression '2d20 kh ' (roll 2d20 with advantage)
  3. app creates a DiceExpression for that roll, with callbacks to application's code
  4. when rolled, any time the DiceExpression needs a dice result, it'll call the application's callback
  5. when app callback gets the event, it'll prompt the user to roll the appropriate die
  6. app gets result from bluetooth device and passes it back
  7. repeat steps 3-5 until all needed dice have been rolled
  8. DiceExpression finishes evaluation and returns all results to the app

@saif-ellafi
Copy link
Author

Hello Steve.

I have implemented 3D Dice Rolling based on this PR. Totally fine if it doesn't follow your principles for the library! Just wanted to share. I'd love to stay in sync with the repo and the future PR too.

Currently I am passing a Custom Dice Roller that implements its own roll() method. The animations are all futures so I definitely needed to roll all sides of the expressions in parallel, and also wait for the animations to finish.

Bluetooth dice and other external metrics collections, stats, and analyrics could work with this PR too

Best regards

@saif-ellafi
Copy link
Author

saif-ellafi commented Jul 5, 2025

And to your example, not exactly. It is not really just a callback, it is rather define the results of the roll().

Even if a Bluetooth dice is rolled (which is not my case by the way), the Bluetooth dice roller would be the one defining the result of the 1d6 roll and not the DefaultDiceRoller I am proposing in this PR!

Then we would have access from the outside to receive your library's instructions and parsing power, but with an external operator acting to take the rolls themselves, rolls, analytics, and whatever else.

Super powerful if you ask me!

@stevesea
Copy link
Collaborator

stevesea commented Jul 5, 2025

I'm still not clear on the use case. I think I'm having a hard time wrapping my head around inverting the rolling from how I usually think of clients using this library.

I'd pictured a nice UI would want detailed results of each roll, and could use that to render the results however it wished. Why does the client application need to implement a roll() method, rather than being notified "a d6 was rolled, the outcome was 2"?

@saif-ellafi
Copy link
Author

saif-ellafi commented Jul 5, 2025

@stevesea because my client is rolling the d6 and is the one setting the result for that specific roll (i.e. the bluetooth roll dice) and not your library! Also because I can't wait to do one roll at a time.

The below example is not using the expressions, but just to show:

ajHsMJ

@stevesea
Copy link
Collaborator

stevesea commented Jul 5, 2025

because my client is rolling the d6 and is the one setting the result for that specific result

I understand that, and I suppose that's the heart of our disconnect here.

In your app, are you simulating rolling the dice as 3D objects on a flat surface, modeling gravity and collision detection to determine the outcome of the roll?

In my mind, I'd pictured the app receiving the results from the library and animates that outcome.

@saif-ellafi
Copy link
Author

Yes, correct! Modeling physics, bluetooth dice, or any other means of rolling, can't take a predefined output.

@saif-ellafi
Copy link
Author

But as mentioned, it is an idea, it is all good if it does not align with your plans and visions! I hope you don't take this the wrong way.

Best regards!

@stevesea
Copy link
Collaborator

stevesea commented Jul 6, 2025

No worries. Just trying to figure out how to best support you.

My current PR already causes lots of conflicts. I'm trying to think through how to enable your use case without a cascade of other changes throughout the API.

@saif-ellafi
Copy link
Author

If you didn't change core architecture of the app, it might be easier to reimplement the PR from scratch.. I can give it another try once the PR build is ready enough and wouldn't take any more changes, and if you confirm the PR is definitely going to Prod (is it objectively better now than before?)

@stevesea
Copy link
Collaborator

stevesea commented Jul 7, 2025

yes, I like the direction of my PR and am continuing progress on that.

Both that PR and this feature will cause breaking changes, so I'd like to incorporate changes from yours too before the next release.

I think I've cracked what you need for this PR. Doing more testing now.

The change I'm working on will let you pass in a DiceRoller :

abstract class DiceRoller {
  /// return an Stream of ints. length == ndice, range: [min,nsides]
  /// duplicates allowed.
  Stream<int> roll({required int ndice, required int nsides, int min = 1});

  /// return an Stream of ints selected from the given vals. length = ndice
  /// duplicates allowed.
  Stream<int> rollVals(int ndice, List<int> vals);
}

I'd like to minimize the awareness client applications have to have of the internal workings and types. The above API felt minimal, but with enough info you can render the dice and return a stream of results.

@saif-ellafi
Copy link
Author

Interesting, I'll have to check out streams. Futures for me where very necessary because I needed to roll more dice while some dice where still rolling (simultaneously roll() would be calling left and right)

Thanks!

@saif-ellafi
Copy link
Author

@stevesea Also please don't forget about the Fate Dice one, using it a lot !! (dF) :D

In fact I was looking for some games who need color distinction (Year Zero engine and many others), to let users distinguish dice of the same type to carry its own "property", like a color or a tag. This way it can be separated.

For example: 2d6 Red (Attack) and 2d6 Blue (Damage), and in one roll be able to get two different sums. But that's another level of complexity that perhaps exceeds the expression. I might have something in my app where players choose which color to roll, but will continue to think about it.

Thanks for the amazing progress! can't wait to give it all a test

@stevesea
Copy link
Collaborator

stevesea commented Jul 7, 2025

fudge dice will roll with Stream<int> rollVals(int ndice, List<int> vals), passing [-1,-1,0,0,1,1] as vals.

@saif-ellafi
Copy link
Author

But how would I know it's fate dice? Perhaps I'll match these lists of values and infer then. Should be reliable I suppose no other dice do the same

@stevesea
Copy link
Collaborator

stevesea commented Jul 7, 2025

rollVals will be passed RolledDie.defaultFudgeVals when rolling fate/fudge. But, that feels brittle to do that comparison up in your app.

There's not a great way to differentiate separate dice sets within an expression (your red&blue example).

Those could be separate DiceExpressions and you'd pass a different DiceRoller that was configured to draw dice with the right color. But, that may not work if you also want to combine the different colored results as part of the expression.

Feels like I could introduce some sort of syntax to pass hints along to the roller.
3d6H[blue,damage] or something.

But, it'll probably be much harder to evaluate the expression like 'red dice should be clamped, but not blue dice" or similar.
If that's needed, then separate DiceExpressions is probably the easier way to go.

@saif-ellafi
Copy link
Author

I think it would be nice already if there was a safety net for adding custom metadata! Sounds like a clever idea if you ask me (and harmless if not used).

One client could benefit from it a lot I suppose to send instructions down the road? With a concrete use case could get clearer, I haven't gotten to that yet but I'm getting it requested a LOT.

I could of course do it without dice parsing (i.e. click and roll different dice colors), but if it worked from expressions too it could get powerful.

But not sure if it is too soon, we could always let the idea sleep for a little.

@stevesea
Copy link
Collaborator

stevesea commented Jul 7, 2025

I want to take some time on cleanup after this influx of changes.

I'm not in a rush to push the new release out right away. I think at a minimum, I could send an enum paramter in those DiceRoller.roll and rollVals calls so that your app has more context about what's being rolled.

@saif-ellafi
Copy link
Author

Totally, no rush at all! I an in the meantime happy with my PR build until the better version comes.

Just fyi : since we have control over the roll function, there is no need to receive any additional metadata from the roles, just to clarify in case of misunderstanding from the previous discussions we had in the past. Roll() could do a lot know about keep track of what happened, I think. Just a thought.

@stevesea
Copy link
Collaborator

stevesea commented Jul 8, 2025

The roll function lets the app do the rolls, but the app isn't involved when it comes to evaluating those rolls in the context of the dice syntax -- dropping/keeping dice, clamping results, etc.

That is, the app doesn't know why it's rolling or what happened the results it gave the library. It rolled one dice pool, and then another... was that second roll from exploding dice? Which of the original results exploded?
Or, highlighting the rolls that were scored as successes or failures. Or, updating the rendered roll after being clamped.

Maybe I need to re-think the API around DiceRoller.roll() -- rather than a Stream, maybe it has to be Stream or some other object.

@saif-ellafi
Copy link
Author

Yes, I am curious how streams could work. Important is that the roll() that dice parser calls isn't blocking, so that left and rights can happen simultaneously.

@stevesea
Copy link
Collaborator

stevesea commented Jul 8, 2025

Ahhh, now I understand that simultaneous flag in your PR.

what side-effect did you see when the left & right futures were waited for sequentially?

@saif-ellafi
Copy link
Author

I was hoping to break less Unit Tests honestly! That's the reason of the flag. But it anyhow broke unit tests because of ordering of large sequences in more complex calls :( also to check if the maths changed, but honestly both work well in terms of logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants