Conversation
Stypox
left a comment
Stypox
left a comment
There was a problem hiding this comment.
Thanks for this nice simple skill that Dicio was definitely missing!
As a future improvement (totally unnecessary for this PR) we could also add some images for coins and dice!
| ctx.parserFormatter?.extractNumber(it)?.mixedWithText | ||
| ?.filterIsInstance<Number>() | ||
| ?.firstOrNull() | ||
| ?.integerValue() | ||
| ?.toInt() |
There was a problem hiding this comment.
If it.isInteger is false, then integerValue() is just 0.
| ctx.parserFormatter?.extractNumber(it)?.mixedWithText | |
| ?.filterIsInstance<Number>() | |
| ?.firstOrNull() | |
| ?.integerValue() | |
| ?.toInt() | |
| ctx.parserFormatter?.extractNumber(it) | |
| ?.first | |
| ?.takeIf { it.isInteger } | |
| ?.integerValue() | |
| ?.toInt() |
Since this appeared often recently, maybe you could change the Number class into a sealed interface with two distinct implementations for integer and decimal, and make integerValue() return null if the value is a decimal. And to go further, I'd also add a firstInteger function to ExtractNumberParams that returns Long? and basically does the operations below. This changes will have to go in https://github.com/Stypox/dicio-numbers
There was a problem hiding this comment.
#384 (comment) also applies here: you could separate specific_dice_roll and specific_random_number sentences and disable those if NumberParserFormatter is not available.
There was a problem hiding this comment.
This is probablt a leftover and should be removed
| import org.stypox.dicio.R | ||
| import org.stypox.dicio.sentences.Sentences.Random | ||
| import org.stypox.dicio.util.getString | ||
| import kotlin.random.Random as KotlinRandom |
There was a problem hiding this comment.
Is this a randomly-seeded RNG, or does it always have the same seed?
| if (results.size == 1) { | ||
| Subtitle(text = ctx.getString(R.string.skill_random_dice_sides, sides)) | ||
| } else { | ||
| Subtitle(text = ctx.getString(R.string.skill_random_dice_sides, sides)) | ||
| Body(text = results.joinToString(" + ") + " = ${results.sum()}") | ||
| } |
There was a problem hiding this comment.
| if (results.size == 1) { | |
| Subtitle(text = ctx.getString(R.string.skill_random_dice_sides, sides)) | |
| } else { | |
| Subtitle(text = ctx.getString(R.string.skill_random_dice_sides, sides)) | |
| Body(text = results.joinToString(" + ") + " = ${results.sum()}") | |
| } | |
| Subtitle(text = ctx.getString(R.string.skill_random_dice_sides, sides)) | |
| if (results.size != 1) { | |
| Body(text = results.joinToString(" + ") + " = ${results.sum()}") | |
| } |
| override fun GraphicalOutput(ctx: SkillContext) { | ||
| Column(verticalArrangement = Arrangement.spacedBy(8.dp)) { | ||
| Headline( | ||
| text = if (results.size == 1) results[0].toString() else results.sum().toString() |
There was a problem hiding this comment.
| text = if (results.size == 1) results[0].toString() else results.sum().toString() | |
| text = results.sum().toString() |
| <string name="failed_to_copy">Failed to copy to clipboard</string> | ||
| <string name="skill_translation_auto">Auto</string> | ||
| <string name="skill_search_duckduckgo_recaptcha">DuckDuckGo did not provide results, asking for a Captcha to be solved</string> | ||
| <string name="skill_random_coin_flip">The coin landed on %1$s</string> |
There was a problem hiding this comment.
I'd rather have two skill_random_coin_flip_heads and skill_random_coin_flip_tails strings, so for languages other than english the strings can be translated naturally in full. Then make the others directly uppercase.
| val total = results.sum() | ||
| return if (results.size == 1) { | ||
| ctx.getString(R.string.skill_random_dice_roll, results[0], sides) | ||
| } else { | ||
| ctx.getString(R.string.skill_random_dice_roll_multiple, total, results.size, sides) | ||
| } |
There was a problem hiding this comment.
| val total = results.sum() | |
| return if (results.size == 1) { | |
| ctx.getString(R.string.skill_random_dice_roll, results[0], sides) | |
| } else { | |
| ctx.getString(R.string.skill_random_dice_roll_multiple, total, results.size, sides) | |
| } | |
| return if (results.size == 1) { | |
| ctx.getString(R.string.skill_random_dice_roll, results[0], sides) | |
| } else { | |
| ctx.getString(R.string.skill_random_dice_roll_multiple, results.sum(), results.size, sides) | |
| } |
2 participants
This PR adds a coin flip/random number/die skill