Skip to content

Conversation

@Bagelboi
Copy link

@Bagelboi Bagelboi commented Feb 2, 2022

This randomizes the caste if either the -caste argument is omitted or if the given caste isn't found

This randomizes the caste if either the -caste argument is omitted or if the given caste isn't found
Copy link
Member

@myk002 myk002 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using a random caste is good when none is specified, but if a caste is specified but not found, the script should throw an error.


if not caste then
error 'Invalid caste.'
local rand_caste = math.random( 0, math.abs(#race.caste-1) )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you write a unit test to show coverage of the case where race.caste has no elements? See the test/ dir for sample unit tests.

error 'Invalid caste.'
local rand_caste = math.random( 0, math.abs(#race.caste-1) )
if not race.caste[rand_caste] then
error 'Creature has no castes or non have been found'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo non -> none

local rand_caste = math.random( 0, math.abs(#race.caste-1) )
if not race.caste[rand_caste] then
error 'Creature has no castes or non have been found'
else
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to have an else clause if the if clause throws an error. Just replace the else with an end and let the assignment happen unconditionally.

@myk002 myk002 changed the title Randomized caste for modtools/transform-unit [waiting on author] Randomized caste for modtools/transform-unit Apr 22, 2022
@myk002
Copy link
Member

myk002 commented Apr 22, 2022

I updated the title of the PR to track the status. Could you remove the [waiting on author] tag when you get back to this script?

@myk002 myk002 changed the title [waiting on author] Randomized caste for modtools/transform-unit Randomized caste for modtools/transform-unit May 9, 2022
@myk002
Copy link
Member

myk002 commented May 10, 2022

@Bagelboi do you have time to address the review comments?

@myk002
Copy link
Member

myk002 commented Jun 3, 2022

ping on this

@myk002
Copy link
Member

myk002 commented Jun 6, 2022

waiting on response from author

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants