dr-scripts
dr-scripts copied to clipboard
[script][common-crafting] New method added to handle checking for sigils and purchasing, if it can.
Leaving guild planned sigils out for now.
Dart, this is a little bit picky but I think it greatly improves code readability if we try to adhere to the Rubocop/Ruby Style Guideline that suggests we should break up methods into internal "paragraphs" that make logical sense to be grouped together.
That can be a little nebulous but in this case some visual spacing really helps. So the main if/else block should have a space between the things before it to set it apart. Perhaps also set apart the array variable assignments from some of the handling below it. Things like that.
That's my final comment on this one unless @Raykyn55 has others, since ya'll are the crafting team.
Thanks for the spacing. Looks much better. I lied about a final comment. @rpherbig notes that this is perhaps the 3rd or 4th place in code where we list the sigils. Shall we turn the sigil lists into a global?
Thanks for the spacing. Looks much better. I lied about a final comment. @rpherbig notes that this is perhaps the 3rd or 4th place in code where we list the sigils. Shall we turn the sigil lists into a global?
I can take a look in doing that tonight and tomorrow night. If it makes sense, sure.
Thanks for the spacing. Looks much better. I lied about a final comment. @rpherbig notes that this is perhaps the 3rd or 4th place in code where we list the sigils. Shall we turn the sigil lists into a global?
I can take a look in doing that tonight and tomorrow night. If it makes sense, sure.
A quick search on sigil in common-crafting
only shows 9 matches all within my new method. Two of those are referencing sigilhunter
.
Thanks for the spacing. Looks much better. I lied about a final comment. @rpherbig notes that this is perhaps the 3rd or 4th place in code where we list the sigils. Shall we turn the sigil lists into a global?
I can take a look in doing that tonight and tomorrow night. If it makes sense, sure.
A quick search on sigil in
common-crafting
only shows 9 matches all within my new method. Two of those are referencingsigilhunter
.
The primary and secondary list of sigils exist separately in attunement, sigilharvest, sigilrecorder, and now here.
You mean like $primary_sigils = ["abolition", "congruence", "induction", "permutation", "rarefaction"]
in common-crafting
at the beginning?
You mean like
$primary_sigils = ["abolition", "congruence", "induction", "permutation", "rarefaction"]
incommon-crafting
at the beginning?
Yes, that's my thought. Then we can utilize those globals directly here, in the 3 other scripts, and in any others.
I will test.
Those are not arrays in those scripts. I mean, not a bad idea to have them global since used in multiple scripts. So, I would need to take PRIMARY_SIGILS_PATTERN = /\b(?:abolition|congruence|induction|permutation|rarefaction) sigil\b/
and then make that into an array to check against the incoming.
The only real reason to keep the check if the sigil is in the list is for when Guild or new sigils are released that are not store bought. I would like to keep the script in the mode that some may not go on sale at the Society store in the future, just like secondaries were not for years.
Those are not arrays in those scripts. I mean, not a bad idea to have them global since used in multiple scripts. So, I would need to take
PRIMARY_SIGILS_PATTERN = /\b(?:abolition|congruence|induction|permutation|rarefaction) sigil\b/
and then make that into an array to check against the incoming.The only real reason to keep the check if the sigil is in the list is for when Guild or new sigils are released that are not store bought. I would like to keep the script in the mode that some may not go on sale at the Society store in the future, just like secondaries were not for years.
I found a way to do it. Testing will begin!
@Dartellum How's this looking? Thanks for adding the sigil globals. I'm showing this PR has conflicts to resolve.
@Dartellum How's this looking? Thanks for adding the sigil globals. I'm showing this PR has conflicts to resolve.
Some updates need added. Working well for me and the few testing from Discord have not posted anything wrong.
Resolved conflicts. Added a catch in case the same challenge occurs. Although, it did not in initial testing but rather safe than sorry.