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
[RDY]Added ability for mutations to spawn items via JSON. #21555
[RDY]Added ability for mutations to spawn items via JSON. #21555
Conversation
|
Seems nice. Maybe in the future, web-producers will be able to harvest their own silk and make clothing out of it? :P. |
|
Interesting property, and I can see this having utility for modding. |
|
If it was possible to generate weapons to toggle like bionic claws or blade, that what also be useful. But unlike this pull request, I can think of no existing mutation that would warrant such a change. Retractable claws for example would likely not hinder the use of hands enough to warrant it. |
|
In its current state it's essentially dead (unused) code. You should make use of it, e.g. port WEB_ROPE and VINES3 to use it. This shows that it's actually working. |
src/mutation_data.cpp
Outdated
| @@ -164,6 +164,7 @@ void mutation_branch::load( JsonObject &jsobj ) | |||
| new_mut.fatigue = jsobj.get_bool("fatigue",false); | |||
| new_mut.valid = jsobj.get_bool("valid", true); | |||
| new_mut.purifiable = jsobj.get_bool("purifiable", true); | |||
| new_mut.spawn_item = _(jsobj.get_string("spawn_item", "").c_str()); | |||
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.
Why do you translate the id of the item? (Remove the _(.c_str()).)
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.
Trial, error, and pasted code. Will change.
src/mutation.h
Outdated
| @@ -114,6 +114,9 @@ struct mutation_branch { | |||
| // Modifier for the rate at which fatigue drops when resting. | |||
| float fatigue_regen_modifier = 0.0f; | |||
|
|
|||
| /** The item, if any, spawned by the mutation */ | |||
| std::string spawn_item; | |||
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.
Should be itype_id instead of std::string.
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.
Admittedly, I know nothing, but that being said: are you sure? I swiped the implementation from fake_item in bionics.cpp/bionics.h and it looks like it passes a string. Or is the bionic solution suboptimal/inadvicable?
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.
it looks like it passes a string.
yes, it does, because itype_id is a dumb typedef for std::string, but hopefully not for long. itype_id will become an alias for string_id<itype_t>. For now we just try to use itype_id where it's meant to be.
src/mutation.cpp
Outdated
| @@ -518,6 +518,12 @@ void player::activate_mutation( const trait_id &mut ) | |||
| print_health(); | |||
| tdata.powered = false; | |||
| return; | |||
| } else if ( !mdata.spawn_item.empty() ) { | |||
| item tmpitem( mdata.spawn_item.c_str() ); | |||
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.
What's with the pointless call to .c_str() here?
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.
If WEB_ROPE and VINES3 were to be migrated, would it be acceptable to lose the unique descriptions and replace with the generic You create a %s.?
|
If WEB_ROPE and VINES3 were to be migrated, would it be acceptable to lose their unique activation messages to be replaced with a generic |
|
If it must be done, assuming you are unable to add a message for it. Maybe if active mutations could have a JSON activation message in general, but that might be fit for a later pull request. |
|
I could add the messages, but it it would be hardcoded, inelegant, and be a step backwards in my attemt to move more things to JSON. I did consider a general activation message, but reached the same conclusion in regards to not overload this PR. |
|
Hardcoded messages would be inelegant, but for now use whatever method suffices for now. |
|
Now this might sound like a stupid question (it is), but how should I replace |
Replace |
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.
Styling issues.
data/json/mutations.json
Outdated
| @@ -3002,7 +3002,8 @@ | |||
| "active" : true, | |||
| "cost" : 30, | |||
| "hunger" : true, | |||
| "thirst" : true | |||
| "thirst" : true, | |||
| "spawn_item" : "rope_30" | |||
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.
Why don't you align "rope_30" to the values above it? Also, "vine_30" below.
src/mutation.cpp
Outdated
| i_add_or_drop( rope ); | ||
| tdata.powered = false; | ||
| } else if (mut == "BURROW"){ | ||
| } else if (mut == "BURROW"){ |
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.
Excess space before else. Also, should be astyled as if( mut == "BURROW" ) {.
src/mutation.cpp
Outdated
| newit = i_add(newit); | ||
| add_msg_if_player(m_info, "%c - %s", newit.invlet == 0 ? ' ' : newit.invlet, newit.tname().c_str()); | ||
| } | ||
| } else if( mut == trait_SELFAWARE ) { |
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.
Excess space before else.
src/mutation.cpp
Outdated
| tdata.powered = false; | ||
| return; | ||
| } else if( mut == trait_SELFAWARE ) { | ||
| print_health(); | ||
| } else if ( !mdata.spawn_item.empty() ) { |
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.
Should be } else if( !mdata.spawn_item.empty() ).
src/mutation.cpp
Outdated
| } else if ( !mdata.spawn_item.empty() ) { | ||
| item tmpitem( mdata.spawn_item ); | ||
| i_add_or_drop( tmpitem ); | ||
| if (mut == "WEB_ROPE") { |
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.
Should be if( mut == "WEB_ROPE") {.
src/mutation.cpp
Outdated
| } else if (mut == "VINES3") { | ||
| add_msg_if_player(_("You detach a vine from your body.")); | ||
| } else { | ||
| add_msg_if_player(_("You create a %s."), tmpitem.display_name().c_str()); |
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.
All add_msg_if_player here and above should have brackets with spaces. Like ( some text here ).
src/mutation_data.cpp
Outdated
| @@ -164,6 +164,7 @@ void mutation_branch::load( JsonObject &jsobj ) | |||
| new_mut.fatigue = jsobj.get_bool("fatigue",false); | |||
| new_mut.valid = jsobj.get_bool("valid", true); | |||
| new_mut.purifiable = jsobj.get_bool("purifiable", true); | |||
| new_mut.spawn_item = jsobj.get_string("spawn_item", ""); | |||
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.
Spaces next to brackets.
src/mutation.cpp
Outdated
| { | ||
| item tmpitem( mdata.spawn_item ); | ||
| i_add_or_drop( tmpitem ); | ||
| if( mut == "WEB_ROPE") { |
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.
One last space, and it's ready to go.
| { | ||
| item tmpitem( mdata.spawn_item ); | ||
| i_add_or_drop( tmpitem ); | ||
| if( mut == "WEB_ROPE" ) { |
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.
Why didn't you jsonize this block as well? With it like this, you still have to edit this file to add a creation message.
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 planned on jsonizing mutation activation messages at some point, but given my inexperience in coding, I tried to limit this PR in scope. Keeping the messages hardcoded felt workable for the time being.
I've added the ability specify "spawn_item" when defining a mutation in JSON. It's not used currently used by any mutations, but WEB_ROPE and VINES3 could be migrated to this system. I would appreciate feedback if I missed something.