New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(Start of) rule system for categorising decks #5736

Merged
merged 11 commits into from Nov 7, 2018

Conversation

Projects
None yet
4 participants
@adambiltcliffe
Collaborator

adambiltcliffe commented Nov 4, 2018

This PR doesn't add any user-visible functionality to decksite, but it gives us a sandbox to play with to work out what features we need from a rule-based deck classification system.

It creates an additional page at /admin/rules visible to admins and demimods that allows you to create a rule for a given archetype and then edit the existing rules. A rule consists of a list of cards which must be included and a list of cards which cannot be included. Multiple rules may exist for the same archetype, e.g. in the case of two different builds or cards with equivalent functionality like Wildfire and Burning of Xinye. This could be more sophisticated, but it seemed a simpler place to start than trying to build a system of arbitrary boolean expressions.

The page then reports on:

  • How many existing decks are covered by a rule
  • How many decks have a conflict (two or more rules apply to them)
  • How many decks have a rule-assigned archetype which is different to their tagged archetype

By playing with this we should be able to work out how plausible it is to get the first number as high as possible and the second and third down to zero.

Currently you can't delete rules or change the archetype they're assigned to, although that should be easy to add. I'm not sure what additional features we would want in this system, although something to do with colour seems likely - classifying a deck as Sultai Control has an obvious first criterion to check that we can't currently do.

@bakert

This comment has been minimized.

Member

bakert commented Nov 5, 2018

Sweeeeeeet!

@bakert

This is pretty sweet. I've made a lot of cosmetic and mostly unimportant suggestions but super glad to see this taking off :D

Show resolved Hide resolved decksite/data/deck.py Outdated
Show resolved Hide resolved decksite/data/deck.py Outdated
def num_classified_decks() -> int:
sql = 'SELECT COUNT(DISTINCT(deck_id)) AS c FROM ({apply_rules_query}) AS applied_rules'.format(apply_rules_query=apply_rules_query())
return (db().select(sql))[0]['c']

This comment has been minimized.

@bakert

bakert Nov 5, 2018

Member

Same comments here about DISTINCT and about db().value(sql)

This comment has been minimized.

@adambiltcliffe

adambiltcliffe Nov 6, 2018

Collaborator

The DISTINCT isn't redundant here if we're in the situation where we have a deck which currently fits multiple rules, right?

This comment has been minimized.

@bakert

bakert Nov 6, 2018

Member

Oh, this is more complicated than I thought, then :D

This comment has been minimized.

@adambiltcliffe

adambiltcliffe Nov 6, 2018

Collaborator

The result set of the SQL contains one row for each (deck_id, rule_id) pair that could apply (it also contains the archetype_id for convenience, but that information can be extracted from the rule_id).

Show resolved Hide resolved decksite/data/rule.py Outdated
Show resolved Hide resolved decksite/data/rule.py Outdated
@@ -0,0 +1,13 @@
CREATE TABLE IF NOT EXISTS rule (

This comment has been minimized.

@bakert

bakert Nov 5, 2018

Member

rule feels pretty generic. archetype_rule? Or does that imply it's a join table of archetypes and rules?

This comment has been minimized.

@adambiltcliffe

adambiltcliffe Nov 5, 2018

Collaborator

Previously you suggested a single-word name for tables that represent a single entity. Of course, arguably a rule is a join of an archetype and a rule, we just don't bother tracking the 'rule' part because it contains zero information and the relationship is one-to-many. I don't know if we're ever sufficiently likely to have anything else called rule that this has any potential ambiguity to make it worth making all the queries even more verbose.

This comment has been minimized.

@bakert

bakert Nov 5, 2018

Member

Yeah, neither thing makes me totally happy. Rule is just such a very general word. But I guess there's not much point in changing it to something longer which is also problematic.

Are any of classification/codification/systematization/codification/allocation/designation/regulation/assignment better? Probably not.

Show resolved Hide resolved decksite/sql/47.sql Outdated
Show resolved Hide resolved decksite/templates/editrules.mustache
Show resolved Hide resolved decksite/templates/editrules.mustache
Show resolved Hide resolved decksite/templates/editrules.mustache
@silasary

This comment has been minimized.

Member

silasary commented Nov 5, 2018

What do rules look like?

@bakert

This comment has been minimized.

Member

bakert commented Nov 6, 2018

I had a whole bunch of second-order thoughts about this. Mostly about "priority" of both rules and cards. And colors. But I'm going to let it all percolate a bit and maybe have a fiddle with what you have built so far. I still think this is going to be super awesome for getting every Cloudy with Sunny Spells or Reliving Ultimatums deck into the right place very-close-to-automatically even if we don't do crazy more-complicated stuff like saying "this looks like a Grixis Control deck" to the first Grixis Control deck of a new season or whatever. It's gonna be GREAT! :D

@silasary

This comment has been minimized.

Member

silasary commented Nov 6, 2018

I posted this in #code a while back, but Gatherling's old system used archetype definitions like this one

The sorter finds all archetypes where the deck matches all Required cards.
It then scores the remaining archetypes, where each Strong is 5 points, each moderate is 3, and each weak is 1.
The archetype with the highest score is then used.

@adambiltcliffe

This comment has been minimized.

Collaborator

adambiltcliffe commented Nov 6, 2018

I'm a bit wary about a scoring/weighting-based system just because there are a lot of parameters to fine-tune and if it's getting the results wrong it's not always clear what the appropriate way to fix it is. We may well need something more fuzzy as this develops but I'd like to start off by seeing how much of the deck-space we can cover with straightforward boolean rules.

@vorpal-buildbot vorpal-buildbot merged commit 9684704 into master Nov 7, 2018

4 checks passed

Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
pdm/automerge Ready to merge
@vorpal-buildbot

This comment has been minimized.

Contributor

vorpal-buildbot commented Nov 7, 2018

Seen on LOGS (created by @adambiltcliffe and merged by @vorpal-buildbot 1 minute and 4 seconds ago) Please check your changes!

@vorpal-buildbot

This comment has been minimized.

Contributor

vorpal-buildbot commented Nov 7, 2018

Seen on PROD (created by @adambiltcliffe and merged by @vorpal-buildbot 1 minute and 9 seconds ago) Please check your changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment