Skip to content

Add TF2 specific player subclass. Add class names and id's to team data #340

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Add TF2 specific player subclass. Add class names and id's to team data #340

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
df4aaee
Add TF2 specific player type. Add class names and id's to team data
p-hennessy Jun 29, 2020
e48d862
Refactor for cleanliness
p-hennessy Jun 29, 2020
f070bc4
Fix names
p-hennessy Jun 29, 2020
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
Prev Previous commit
Fix names
  • Loading branch information
@p-hennessy
p-hennessy committed Jun 29, 2020
commit f070bc4ef9ee0d428c60bb815c97027a708af838
8 changes: 4 additions & 4 deletions addons/source-python/packages/source-python/players/engines/orangebox/tf.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ def team_name(self):

@team_name.setter
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than having multiples team_name methods decorated as getter and setter I think it would be better to follow the overall design of the other properties across the whole project where they are distinctly available as methods as well. For example, get_team_name and set_team_name bound to the team_name property.

def team_name(self, team_name):
if team_id not in self._teams_names:
if team_name not in self._teams_names:
raise ValueError(f"Invalid team_name: {team_name}")

self.team = self._teams_names[team_name]
Expand All @@ -42,8 +42,8 @@ def player_class_name(self):

@player_class_name.setter
def player_class_name(self, class_name):
if team_id not in self._class_names:
if class_name not in self._class_names:
Copy link
Contributor

Choose a reason for hiding this comment

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

The dictionary here is queried twice (once through __contains__, then another through __getitem__). A good practice is to query the dictionary only once, for example using dict.get:

v = d.get(k, None)

# If k wasn't in d, let's raise
if v is None:
    raise

...

This would ensure the dictionary is looked up only once rather than twice, etc.

raise ValueError(f"Invalid class_name: {class_name}")

# Does this need to be desired_player_class or should there be a setter for desired class?
self.player_class = self._class_names[team_name]
# Does this need to be desired_player_class or should there b a setter for desired class?
self.player_class = self._class_names[class_name]