-
Notifications
You must be signed in to change notification settings - Fork 298
Changes provided by Storan and ashugar #476
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
Conversation
Changes for helpers.js and stanek.js tailing issue provided by ashugar
alainbryden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution!
| processId ??= ns.pid | ||
|
|
||
| /* no-tail option used in corresponding script */ | ||
| if (close){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed adding this new optional "close" parameter to the function definition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, the comment doesn't really make sense to me, perhaps you can explain
| const trainingReserveFile = '/Temp/sleeves-training-reserve.txt'; | ||
| const works = ['security', 'field', 'hacking']; // When doing faction work, we prioritize physical work since sleeves tend towards having those stats be highest | ||
| const trainStats = ['str', 'def', 'dex', 'agi']; | ||
| const trainStats = ['strength', 'defense', 'dexterity', 'agility']; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@catloversg - you were the one to change this from long-form to 3-character codes back in your PR #467 here:
e3f9227#diff-4ca659d70c6b95a5c56e834df709c9166bd8c5d5d8f5c0940936c5ab59d4cac3R35
But now these folks are saying it needs to go back. Is this so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm seeing what's wrong here.
The sleeve stat name is named using the long form, e.g.:
ns.sleeve.getSleeve(1).skills['strength']
But when we go to the gym, the stat needs to be shortened to 3 characters:
ns.sleeve.setToGymWorkout(..., ..., 'str')
With the game currently accepting "strength" as the stat to train, everything is consistent, so I was using the same enum values both to get the stats, and to name the stat to train.
In 3.0, I won't be able to do that anymore, but it seems I can simply pass the first 3 characters of the trainStats enum value to the gym workout function and get away with it,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. Gym APIs use the short form, while other APIs use the long form. Using slice(0, 3) like you do is fine. It's not ideal, but I don't think we will change these values again in the near future.
|
I appreciate your contribution to light a fire under my ass, really! In this case, I'm going to have to reject and submit separate fixes, because each needs a bit of tweaking:
Again, appreciate the contributions, always, but in this case I'll be closing and fixing both these issues a bit differently. |
Changes for sleeve.js training issue provided by Storan ( #475 )
Changes for helpers.js and stanek.js tailing issue provided by ashugar ( #462 )