Skip to content
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

Option to yell sentences #21495

Merged
merged 3 commits into from
Aug 22, 2017
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 28 additions & 7 deletions src/game.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10872,24 +10872,45 @@ void game::chat()
}

uimenu nmenu;
nmenu.text = std::string( _("Who do you want to talk to?") );

nmenu.text = std::string( _( "Who do you want to talk to or yell?" ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be: "Who do you want to talk to or yell at?"

Otherwise, this reads (without the first clause): "Who do you want to yell?"

Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't checked what the actual UI looks like, but you may not even need to add the "Yell" clause to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That text is displayed whether there are NPCs or not, we could add a check to it, but it would automagically tell the player if there are NPCs around.

int i = 0;

for( auto &elem : available ) {
nmenu.addentry( i++, true, MENU_AUTOASSIGN, ( elem )->name );
}

nmenu.return_invalid = true;
nmenu.addentry( i++, true, 'a', _( "Yell" ) );
nmenu.addentry( i++, true, 'q', _( "Cancel" ) );

int yell, yell_sentence, cancel;

nmenu.addentry( yell = i++, true, 'a', _( "Yell" ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

This kind of assignment is ugly and confusing. An enum could have been used here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I googled around if you can assign an enum's value can be assigned when called, and it seems that they are constant expression. So I don't know how enums can be used in this case.

I could comment more all the code, to it is at least less confusing, but between that I'm a noob at programming and that this method didn't have much comments, I don't really know when to comment and how much to comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

and it seems that they are constant expression.

They certainly are. I meant getting rid of any assignments entirely. See this function as a good example of what I'm talking about. You can use negative values to avoid overlapping with indexes of NPCs:

    enum class choices {
        just_yell = INT_MIN,
        yell_something,
        ...
    };

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using enum class doesn't seems to work (had to I cast it to int for ui.addentry() to accept it) which for some reason it takes it as zero and one. Using non-class enums works only for the first enum but not for the second as it is taken as 1.

I'm manually assigning them both to -2 and -1 respectively. And it seems that the addentry() and query() both seems to support negative integers.

Any advise?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are negative numbers sorted after non-negative by the game's engine? If no, what you propose won't work.

There is no sorting done by the "game engine" (what do you actually mean with that?) at all. The menu displays the options in the order they have been added (actually: the order in which they appear in uimenu::entries).

nmenu.addentry( yell_sentence = i++, true, 'b', _( "Yell a sentence" ) );
nmenu.addentry( cancel = i++, true, 'q', _( "Cancel" ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you've touched this line, could you remove it and update the condition below (see #12112)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm sorry, but I don't know exactly what you want me to do, mind explaining it please?

Copy link
Contributor

Choose a reason for hiding this comment

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

Most of our menus are escapable i.e. they can be closed using Esc button, which makes options like "Cancel" redundant. You can simply remove it to make this particular menu more consistent with the others.


nmenu.query();
if( nmenu.ret < 0 || nmenu.ret > (int)available.size() ) {
if( nmenu.ret < 0 || nmenu.ret == cancel ) {
return;
} else if( nmenu.ret == (int)available.size() ) {
} else if( nmenu.ret == yell ) {
u.shout();
} else {
} else if( nmenu.ret == yell_sentence ) {
std::string popupdesc = string_format( _( "Enter a sentence to yell" ) );
string_input_popup popup;
popup.title( string_format( _( "Yell a sentence" ) ) )
.width( 64 )
.description( popupdesc )
.identifier( "sentence" )
.max_length( 128 )
.query();

std::string sentence = popup.text();
add_msg( _( "You yell %s" ), sentence.c_str() );
u.shout();

} else if( nmenu.ret <= ( int )available.size() ) {
available[nmenu.ret]->talk_to_u();
} else {
return;
Copy link
Member

Choose a reason for hiding this comment

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

Bad indent.

}

u.moves -= 100;
Expand Down