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

ship_view.py: Fix a few things, best I can figure out #144

Merged
merged 2 commits into from
Jan 16, 2025

Conversation

stephengtuggy
Copy link
Contributor

@stephengtuggy stephengtuggy commented Jan 15, 2025

Code Changes:

Issues:

  • I don't think a ticket has been filed yet for the errors this PR addresses

Purpose:

  • What is this pull request trying to do? Fix some errors being thrown in ship_view.py
  • What release is this for? 0.10.x
  • Is there a project or milestone we should apply this to? 0.10.x

@stephengtuggy stephengtuggy added this to the 0.10.x milestone Jan 15, 2025
@stephengtuggy stephengtuggy self-assigned this Jan 15, 2025
Copy link
Contributor

@evertvorster evertvorster left a comment

Choose a reason for hiding this comment

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

Looks really good.
Please merge at the same time as the other fixes some stuff PR in engine.

Copy link
Member

@BenjamenMeyer BenjamenMeyer left a comment

Choose a reason for hiding this comment

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

LGTM - left a comment on something that we could do a little better now that we're using Py3 - probably a little more efficient than the older methods too.

try:
fl = float(ship_stats[key])/divider
except KeyError:
print('KeyError in get_dbl: ', key, ' not found in ship_stats')
Copy link
Member

Choose a reason for hiding this comment

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

FYI - since we're using Py3 we can use f-Strings here we makes formatting really nice and easy:

print(f'KeyError in get_dbl: {key} not found in ship_stats')

Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be f"blurb" and not f'blurb'?

Copy link
Member

Choose a reason for hiding this comment

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

@royfalk it doesn't really matter which; typically you use one if you want to use the other:

f"blurb 'b' "
f'blurb "b" '

they both denote a string.

Copy link
Contributor

@royfalk royfalk left a comment

Choose a reason for hiding this comment

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

LGTM

@stephengtuggy stephengtuggy merged commit dad7942 into vegastrike:master Jan 16, 2025
11 checks passed
@stephengtuggy stephengtuggy deleted the fix/ship_view branch January 16, 2025 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants