Skip to content

Another Countables Pass - RFC #13204

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

SomeTroglodyte
Copy link
Collaborator

Each of the 4 commits should be read individually, and each could be approved or rejected individually. I'm not drafting this because I had a slight prejudice you tend to overlook those.

Top to bottom:

  • cb6f247 is almost necessary by now, but definitely requires brains to dissect and evaluate. Idea was to lay a groundwork to offer a "does not fit parameter type countable, but may be an incorrect use of \"${countable.text}\"." kind of output. Haven't yet invested many seconds into a design how to actually integrate that. Also to think about: Countables.TileResource having no discernible pattern to them is what enforces a lot of the complications. To drop or not to drop (in favour of "[resourceFilter] Resources")? I fear we can't, but without that beast it would be so much easier...
  • c9f27b8 is super cute, but maybe way over the top 😄
  • 6d95ea3 is just the demo how I'd see the integration: Keep all those Token, Operator, Operand classes - and caches - out of there. Right?
  • cb6f247 Is minor code quality stuff.

Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

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

Successfully merging this pull request may close these issues.

2 participants