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

add draw item query when wielding holster #23116

Merged
merged 6 commits into from Mar 3, 2018

Conversation

Projects
None yet
2 participants
@ymber
Copy link
Contributor

ymber commented Mar 3, 2018

closes #19865

Queries whether to draw the item from a holster you attempt to wield if it contains anything.


// Query whether to draw an item from a holster when attempting to wield the holster
if( target.get_use( "holster" ) && ( target.contents.size() > 0 ) ) {
if( query_yn( string_format( _( "Draw %s from %s?" ), target.get_contained().tname().c_str(), target.get_contained().tname().c_str()) ) ) {

This comment has been minimized.

@illi-kun

illi-kun Mar 3, 2018

Member

Second string is wrong, "Draw Beretta M9 from Beretta M9?"

This comment has been minimized.

@illi-kun

illi-kun Mar 3, 2018

Member

BTW, this line itself is way too long, please target on 100 symbols max in one line (see astyle rules for details).

ymber added some commits Mar 3, 2018


// Query whether to draw an item from a holster when attempting to wield the holster
if( target.get_use( "holster" ) && ( target.contents.size() > 0 ) )
{

This comment has been minimized.

@illi-kun

illi-kun Mar 3, 2018

Member

I don't want to be annoying and don't want to discourage you from other PRs but it still wrong formatted. { should be on a previous line (see an example right above), and

if( target.get_use( "holster" ) && ( target.contents.size() > 0 ) )
{
if( query_yn( string_format( _( "Draw %s from %s?" ),
target.get_contained().tname().c_str(),

This comment has been minimized.

@illi-kun

illi-kun Mar 3, 2018

Member

... and we need one more space here and on a line below.

If you feel it takes to much efforts to get minor improve then simply ignore it, it will not be a blocker, at least for me.

@ymber

This comment has been minimized.

Copy link
Contributor Author

ymber commented Mar 3, 2018

Current styling is what I got from the astyle instructions in CODE_STYLE.md. Is that outdated?

@illi-kun

This comment has been minimized.

Copy link
Member

illi-kun commented Mar 3, 2018

I've compared .astylerc with CODE_STYLE.md and it seems they're identical. Why do you think it's outdated?

@ymber

This comment has been minimized.

Copy link
Contributor Author

ymber commented Mar 3, 2018

I got it. It was formatting differently because I cut the new section out and astyled it on its own in a different file. Updated it as astyled properly.

@illi-kun

This comment has been minimized.

Copy link
Member

illi-kun commented Mar 3, 2018

Great, thanks.

@illi-kun illi-kun self-assigned this Mar 3, 2018

@illi-kun illi-kun merged commit f5a5be1 into CleverRaven:master Mar 3, 2018

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
gorgon-ghprb Build triggered. sha1 is merged.
Details

@ymber ymber deleted the ymber:wieldholster branch May 21, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.