Closed andrew-fleming closed 1 year ago
So I think this is a good first iteration of the evaluations test suite. Some things to note:
battleProcessor
should be in another PR. We need to be able to pass the rules
argument to it and get rid of the local rules
dictionary. I know you had some ideas on how not to bloat the objects, so I suppose we can defer those tests for now?I should also mention, if we merge this, we have to manually go to battleProcessor
and adjust the rules
dictionary accordingly every time we want a different set of rules. I'll default the rules to standard
so it doesn't get weird
I prefer the SCREAMS because they're very readable, but to @dustinstacy's credit, they're really just variables inside a big function and we're likely not going to use them anywhere else
Exactly what I found in my 'standards/best practices' refresher. Anytime you have a global variable as you mentioned, it's best to declare it in SSC, otherwise it's the JS caMels.
I know you had some ideas on how not to bloat the objects, so I suppose we can defer those tests for now?
Agreed, best to hang on to that for when we implement the battle context in the next 'release'
I should also mention, if we merge this, we have to manually go to
battleProcessor
and adjust therules
dictionary accordingly every time we want a different set of rules. I'll default the rules tostandard
so it doesn't get weird
Gotcha. So essentially altering the the rules does nothing but, change the way the evaluations are handled, correct? Asking because we could potentially attach the rule set to the opponent object for now to give it a little spice this release before creating the context as mentioned for the next.
Clearly going to involve a little more that it sounds given we need to instruct and communicate the different rules to the user, but it'll have to happen eventually anyways.
So essentially altering the the rules does nothing but, change the way the evaluations are handled, correct?
Yes, exactly
Asking because we could potentially attach the rule set to the opponent object for now to give it a little spice this release before creating the context as mentioned for the next.
We could do that! I might softly vote against it though because it sounds like it'll add redundant work. Idk you can probably talk me into it :)
Resolves #16.
~This PR needs further tests and will propose to change the
processStandardBattles
name.And for whatever reason, I can't set the PR as a draftApparently, drafting PRs is not available for private repos within the free github membership~