feat: getTech now accepts TitleCase or camelCase
#4010
Conversation
* update techOptions to look for `TitleCase`/`camelCase` options * remove all possible `toTitleCase` calls from player.js * remove deprecated usage of Tech as Component * add a unit test to verify that registerTech works * change defaultTech_ to defaultTechOrder_
| @@ -840,6 +843,8 @@ class Player extends Component { | |||
| techOptions[props.getterName] = this[props.privateName]; | |||
| }); | |||
|
|
|||
| assign(techOptions, this.options_[titleTechName]); | |||
There was a problem hiding this comment.
Going to add some unit tests to assign to find out, will open another PR with those
There was a problem hiding this comment.
See #4014 for tests, seems that undefined/null/empty will do nothing to an existing object
| // Support old behavior of techs being registered as components. | ||
| // Remove once that deprecated behavior is removed. | ||
| if (!TechComponent) { | ||
| TechComponent = Component.getComponent(techName); |
There was a problem hiding this comment.
I guess we should throw an error in this case?
There was a problem hiding this comment.
I will improve the error handing here
There was a problem hiding this comment.
I don't know if we show check getComponent at all. I think we should just throw an error if TechComponent/TechClass does not exist. We could check getComponent here but I think that we should be able to have a Tech and a Component with the same name (as weird as that would be)
There was a problem hiding this comment.
yeah, I meant to just throw if TechClass isn't available. We shouldn't try to call getComponent anymore.
brandonocasey
force-pushed
the
feat/title-case-get-tech
branch
from
dabd942 to
ebd909e
Compare
brandonocasey
force-pushed
the
feat/title-case-get-tech
branch
from
ebd909e to
0049ea2
Compare
Description
getTechnow works withTitleCaseandcamelCasetech namesFixes #3986
Specific Changes proposed
TitleCase/camelCaseoptionstoTitleCasecalls from player.jsRequirements Checklist