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

Show file tree
Hide file tree
Changes from all commits
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
11 changes: 11 additions & 0 deletions addons/source-python/data/source-python/teams/tf.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,14 @@ un = 0
spec = 1
red = 2
blue = 3

[classes]
scout = 1
sniper = 2
soldier = 3
demoman = 4
medic = 5
heavy = 6
pyro = 7
spy = 8
engineer = 9
49 changes: 49 additions & 0 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
@@ -0,0 +1,49 @@
# ../players/engines/orangebox/cstrike.py

"""Provides Team Fortress 2 specific Player based functionality."""

# =============================================================================
# >> IMPORTS
# =============================================================================
# Source.Python Imports
from . import Player as _Player
from configobj import ConfigObj
from core import GAME_NAME
from paths import SP_DATA_PATH

# =============================================================================
# >> CLASSES
# =============================================================================
class Player(_Player):
caching = True

# Team
_team_data = ConfigObj(SP_DATA_PATH / 'teams' / GAME_NAME + '.ini', unrepr=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to parse that file again here, as it is already parsed into ../players/teams.py.


_teams_names = _team_data.get('names')
Copy link
Contributor

Choose a reason for hiding this comment

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

This mapping is already available as players.teams.teams_by_name and should really just be imported from there rather than queried again from the INI file.

_class_names = _team_data.get('classes')

@property
def team_name(self):
teams_by_number = dict(zip(self._teams_names.values(), self._teams_names.keys()))
return teams_by_number[self.team]

@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_name not in self._teams_names:
raise ValueError(f"Invalid team_name: {team_name}")

self.team = self._teams_names[team_name]

@property
def player_class_name(self):
class_by_number = dict(zip(self._class_names.values(), self._class_names.keys()))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is redundant to rebuild that mapping every calls, and it should really just be globalized into the players.teams module like the teams mapping are.

return class_by_number[self.player_class]

@player_class_name.setter
def player_class_name(self, class_name):
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 b a setter for desired class?
self.player_class = self._class_names[class_name]