Only give message about sleeping aid once #37404
Conversation
esotericist
added
<Bugfix>
|
I'll give this a spin when I get back from errands. good work on murdering more auto. |
esotericist
added
the
[C++]
There was a problem hiding this comment.
Prior to this change, the function base_comfort_value did two things: calculating the level of comfort (its main purpose) and printing the message (a non-obvious side effect). Now it does three: the side effect starts depending on the flag.
I'd suggest a slightly cleaner approach. You can return a plain struct with the result of the "query" instead. Something along these lines:
struct comfort_response_t {
comfort_level level;
const item* aid;
};So that the function becomes pure, and you can describe the result of the query in the calling code as needed (for example, print different descriptions for different callers, or do something fancy with the acquired information).
| add_msg_if_player( m_info, _( "You use your %s for comfort." ), items_it.tname() ); | ||
| if( message ) { | ||
| add_msg_if_player( m_info, _( "You use your %s for comfort." ), items_it.tname() ); | ||
| } |
There was a problem hiding this comment.
It would be nice to also deduplicate this snippet.
src/character.cpp
Outdated
| auto items = g->m.i_at( p ); | ||
| for( auto &items_it : items ) { | ||
| const map_stack items = g->m.i_at( p ); | ||
| for( const item &items_it : items ) { | ||
| if( items_it.has_flag( "SLEEP_AID" ) ) { | ||
| // Note: BED + SLEEP_AID = 9 pts, or 1 pt below very_comfortable | ||
| comfort += 1 + static_cast<int>( comfort_level::slightly_comfortable ); |
There was a problem hiding this comment.
I can't test it right now, but I'm pretty sure there's a bug in here. One can put one pillow inside a vehicle and yet another under it, thus getting the bonus from both pillows (and two messages as a symptom).
src/character.cpp
Outdated
| { | ||
| // Comfort of sleeping spots is "objective", while sleep_spot( p ) is "subjective" | ||
| // As in the latter also checks for fatigue and other variables while this function | ||
| // only looks at the base comfyness of something. It's still subjective, in a sense, | ||
| // as arachnids who sleep in webs will find most places comfortable for instance. | ||
| int comfort = 0; | ||
|
|
||
| comfort_response_t comfort_response; | ||
| comfort_response.aid = &item( "null" ); |
There was a problem hiding this comment.
This stores a pointer to a temporary object. This pointer becomes invalid right after this statement. I suggest you initialise it with a nullptr and check for that.
There was a problem hiding this comment.
Yep, it should be fixed now.
| { | ||
| // Comfort of sleeping spots is "objective", while sleep_spot( p ) is "subjective" | ||
| // As in the latter also checks for fatigue and other variables while this function | ||
| // only looks at the base comfyness of something. It's still subjective, in a sense, | ||
| // as arachnids who sleep in webs will find most places comfortable for instance. | ||
| int comfort = 0; | ||
|
|
||
| comfort_response_t comfort_response; |
There was a problem hiding this comment.
Now the pointer is uninitialised. Reading it is undefined behaviour.
Summary
SUMMARY: Bugfixes "Only give message about sleeping aid once"Purpose of change
Fixes #37121
Describe the solution
If
base_comfort_valueis called from update_needs don't print a messageDescribe alternatives you've considered
Testing
Try to sleep on a pillow
Before fix get tons of message
After fix get only one message
Additional context