5
\$\begingroup\$

I'm posting here to ask for some help improving myself using python since I'm fairly new to it and come from C#.

The main functionality of this bot is parsing the Wargaming API for player related information.

Project structure:

project
|    main.py
└───ENV
│   
└───data
    |    classes.py
    |    dbcontext.py
    |    getStats.py
    |    secret.py
    |    log.py
    |    updateDb.py
    |    updateDbData.json

My Main:

####### General imports #######
import asyncio
import logging
####### Discord imports #######
import discord
from discord.ext import commands
####### Data imports #######
import data.translations as trans
from data import dbcontext
from data import getStats as GetStats
from data import log, secret
from data.classes import Config, ErrorType, Player, ReturnVal, Ship, Stats


"""
WoWs-Stats Bot.
Creator Fuyune

Uses Discord.py framework by Rapptz
"""

####### Bot Basic Configuration #######
bot = commands.Bot(command_prefix="!")
bot.remove_command('help')

####### err.log handler #######
logger = logging.getLogger('discord')
logger.setLevel(logging.ERROR)
handler = logging.FileHandler(filename='err.log', encoding='utf-8', mode='a')
handler.setFormatter(logging.Formatter(
    '%(asctime)s:%(levelname)s:%(name)s: %(message)s'))
logger.addHandler(handler)

####### Global Vars #######
configs = []
regions = ["eu", "ru", "na", "asia"]
langs = ["de", "en", "pl", "tr"]  # remember adding to imports too
translations = [trans.de, trans.en, trans.pl, trans.tr]


####### Bot Events #######
@bot.event
async def on_ready():
    log.writeLog("init", "Ready")
    print("Bot Started")
    configs = []
    for x in bot.guilds:
        configs.append(dbcontext.getConfig(x.id))
        print("Connected to: " + x.name)
        log.writeLog("Connected", x.name)


@bot.event
async def on_guild_join(guild):
    log.writeLog("Connected", guild.name)
    configs = []
    for x in bot.guilds:
        configs.append(dbcontext.getConfig(x.id))


@bot.command()
@commands.has_any_role('Stats-Bot')
async def statsRegion(ctx, *args):
    if args[0] not in regions:
        await ctx.send("Supported regions are: " + " ".join(regions[0:]))
    else:
        cfg = dbcontext.getConfig(ctx.guild.id)
        if args[0] == "na":
            cfg.region = "com"
        else:
            cfg.region = args[0]
        retval = dbcontext.updateConfig(cfg)
        if retval == ReturnVal.SUCCESS:
            await ctx.send("Region changed to: " + args[0])
        else:
            await ctx.send("Failed to change region.")


@bot.command()
@commands.has_any_role('Stats-Bot')
async def statsLang(ctx, *args):
    if args[0] not in langs:
        await ctx.send("Supported languages are: " + " ".join(langs[0:]))
    else:
        cfg = dbcontext.getConfig(ctx.guild.id)
        cfg.language = args[0]
        retval = dbcontext.updateConfig(cfg)
        if retval == ReturnVal.SUCCESS:
            await ctx.send("Language changed to: " + args[0])
        else:
            await ctx.send("Failed to change Language.")


@bot.command()
@commands.has_any_role('Stats-Bot')
async def statsAddAsn(ctx, *args):
    import re
    name = ""
    asn = ""
    if len(args) == 2:
        name = re.sub("[\"]", '', args[0])
        asn = re.sub("[\"]", '', args[1])
        result = dbcontext.addAsn(name, asn)
        if result == ReturnVal.SUCCESS:
            await ctx.send("Alternate shipname {an} for {on} was added".format(an=asn, on=name))
        elif result == ReturnVal.DOUBLE:
            await ctx.send("Alternate shipname already set")
        else:
            await ctx.send("An error has occured, please contact Owner")
    else:
        await ctx.send("Usage: !statsAddAsn \"Original Shipname\" \"Alternate Shipname\". the \" are Important and mandatory. ")


@bot.command()
async def stats(ctx, *args):
    if ctx.message.guild is not None:
        config = dbcontext.getConfig(ctx.guild.id)
        # if (len(args) == 0) or (len(args) == 1 and args[0].lower() == "help"):
        if len(args) == 0 or args[0].lower() == "help":
            await writeHelp(ctx, config)
        elif len(args) == 1 and args[0] != "help":
            playerObject = GetStats.getPlayer(config, args[0])
            if playerObject == ErrorType.UNKNOWN_PLAYER:
                await writeError(ctx, config, playerObject)
            else:
                playerStats = GetStats.getStats(config, playerObject)
                if playerStats == ErrorType.HIDDEN_STATS:
                    await writeError(ctx, config, playerStats)
                elif playerStats == ErrorType.UNKNOWN_STATS:
                    await writeError(ctx, config, playerStats)
                elif playerStats == ErrorType.SERVER_ERROR:
                    await writeError(ctx, config, playerStats)
                elif playerStats == ErrorType.INTERNAL_ERROR:
                    await writeError(ctx, config, playerStats)
                else:
                    await writeAnswer(ctx, config, playerObject, playerStats)
        elif len(args) >= 2:
            playerObject = GetStats.getPlayer(config, args[0])
            if playerObject == ErrorType.UNKNOWN_PLAYER:
                await writeError(ctx, config, playerObject)
            else:
                ship = dbcontext.getShip(" ".join(args[1:]))
                if ship == ErrorType.UNKNOWN_SHIP:
                    await writeError(ctx, config, ship)
                elif ship == ErrorType.INTERNAL_ERROR:
                    await writeError(ctx, config, ship)
                else:
                    shipStats = GetStats.getStats(config, playerObject, ship)
                    if shipStats == ErrorType.HIDDEN_STATS:
                        await writeError(ctx, config, shipStats)
                    elif shipStats == ErrorType.UNKNOWN_STATS:
                        await writeError(ctx, config, shipStats)
                    else:
                        await writeAnswer(ctx, config, playerObject, ship, shipStats)
    else:
        await ctx.send("**I'm not allowed to answer in Private messages.**")


@bot.command()
async def statsr(ctx, *args):
    import re
    if ctx.message.guild is None:
        await ctx.send("**I'm not allowed to answer in Private messages.**")
    else:
        config = dbcontext.getConfig(ctx.guild.id)
        # regex to check for valid season
        regex = re.compile("^(![1-9][1]|![1-9])|(!s[1-4])$", re.IGNORECASE)
        if (len(args) == 0) or (len(args) == 1 and args[0].lower() == "help"):
            await writeHelp(ctx, config)
        elif regex.match(args[0]):
            await writeError(ctx, config, ErrorType.UNKNOWN_SEASON)
        elif len(args) == 2 and args[0] != "help":
            season = convertSeason(args[0])
            playerObject = GetStats.getPlayer(config, args[1])
            if playerObject == None or playerObject.id == 0:
                await writeError(ctx, config, ErrorType.UNKNOWN_PLAYER)
            else:
                playerStats = GetStats.getRankedStats(
                    config, playerObject, season)
                if playerStats.hidden:
                    await writeError(ctx, config, ErrorType.HIDDEN_STATS)
                elif playerStats.damage == 0:
                    await writeError(ctx, config, ErrorType.UNKNOWN_STATS)
                else:
                    await writeAnswer(ctx, config, playerObject, playerStats)
        elif len(args) > 2:
            season = convertSeason(args[0])
            playerObject = GetStats.getPlayer(config, args[1])
            if playerObject == None or playerObject.id == 0:
                await writeError(ctx, config, ErrorType.UNKNOWN_PLAYER)
            else:
                ship = dbcontext.getShip(" ".join(args[2:]))
                if ship.id == 0:
                    await writeError(ctx, config, ErrorType.UNKNOWN_SHIP)
                else:
                    shipStats = GetStats.getRankedStats(
                        config, playerObject, season, ship)
                    if shipStats.hidden:
                        await writeError(ctx, config, ErrorType.HIDDEN_STATS)
                    elif shipStats.damage == 0:
                        await writeError(ctx, config, ErrorType.UNKNOWN_STATS)
                    else:
                        await writeAnswer(ctx, config, playerObject, ship, shipStats)
        else:
            await writeHelp(ctx, config)

####### Helper Functions #######
async def writeAnswer(ctx, config, *args):
    player = None
    ship = None
    stats = None
    embed = None
    translation = next(
        _cls for _cls in translations if _cls.__name__ == config.language)
    for arg in args:
        if isinstance(arg, Player):
            player = arg
        elif isinstance(arg, Ship):
            ship = arg
        elif isinstance(arg, Stats):
            stats = arg
    if player != None and stats != None and ship != None:
        color = getColor(stats.avgWins)
        embed = discord.Embed(title=translation.title.format(username=player.name), url=GetStats.getPlayerLink(
            config, player), description=translation.description.format(shipname=ship.name), color=color)
        embed.set_author(name="WoWs-Stats-Bot",
                         url="https://github.com/De-Wohli")
        embed.set_thumbnail(url=ship.url)
        embed.add_field(name=translation.battles,
                        value=stats.battles, inline=True)
        embed.add_field(name=translation.avgDamage,
                        value=stats.avgDamage, inline=True)
        embed.add_field(name=translation.winrate,
                        value="{:.2f}%".format(stats.avgWins), inline=True)
        embed.set_footer(text=translation.footer)
    elif player != None and stats != None and ship == None:
        color = getColor(stats.avgWins)
        embed = discord.Embed(title=translation.title.format(username=player.name), url=GetStats.getPlayerLink(
            config, player), description=translation.general, color=color)
        embed.set_author(name="WoWs-Stats-Bot",
                         url="https://github.com/De-Wohli")
        embed.add_field(name=translation.battles,
                        value=stats.battles, inline=True)
        embed.add_field(name=translation.avgDamage,
                        value=stats.avgDamage, inline=True)
        embed.add_field(name=translation.winrate,
                        value="{:.2f}%".format(stats.avgWins), inline=True)
        embed.set_footer(text=translation.footer)
    if embed != None:
        await ctx.send(embed=embed)


async def writeHelp(ctx, config):
    color = discord.Color.teal()
    translation = next(
        _cls for _cls in translations if _cls.__name__ == config.language)
    embed = discord.Embed(title=translation.helpHeader,
                          description=translation.helpDescription, color=color)
    embed.set_author(name="WoWs-Stats-Bot")
    embed.add_field(name="!stats [player]",
                    value=translation.helpPlayer, inline=False)
    embed.add_field(name="!stats [player] [shipname]",
                    value=translation.helpShip, inline=False)
    embed.add_field(name="!statsr [season] [player]",
                    value=translation.helpRanked, inline=False)
    embed.add_field(name="!statsr [season] [player] [shipname]",
                    value=translation.helpSRanked, inline=False)
    embed.set_footer(text="This bot was made by Fuyu_Kitsune")
    await ctx.send(embed=embed)


async def writeError(ctx, config, errorType):
    color = discord.Color.dark_teal()
    translation = next(
        _cls for _cls in translations if _cls.__name__ == config.language)
    errorText = translation.error[errorType.value]
    embed = discord.Embed(title="Error", description=errorText, color=color)
    embed.set_author(name="WoWs-Stats-Bot")
    embed.set_footer(text=translation.footer)
    await ctx.send(embed=embed)


def getColor(value):
    if value <= 40:
        return discord.Colour.red()
    elif value > 40 and value <= 45:
        return discord.Colour.orange()
    elif value > 45 and value <= 50:
        return discord.Colour.gold()
    elif value > 50 and value <= 53:
        return discord.Colour.green()
    elif value > 53 and value <= 56:
        return discord.Color.dark_green()
    elif value > 56 and value <= 60:
        return discord.Color.teal()
    elif value > 60 and value <= 66:
        return discord.Color.purple()
    elif value > 66:
        return discord.Colour.dark_purple()


def convertSeason(value):
    season = value
    if season == "s1":
        season = "101"
    elif season == "s2":
        season = "102"
    elif season == "s3":
        season = "103"
    elif season == "s4":
        season = "104"
    return season


####### Run Bot #######
bot.run(secret.Secret.token)

This being the main method, here is where the parsing of the command takes place. The usual command is !stats [playername] [shipname] My primary concerns here are the async def stats(ctx, *args): and async def statsr(ctx, *args): functions.

data/classes.py:

f####### Imports #######
from enum import Enum


class Ship:
    def __init__(self, id=0, name="", url=""):
        self.id = id
        self.name = name
        self.url = url

    def __eq__(self, other):
        return self.name == other


class Player:
    def __init__(self, id=0, name="", code="404"):
        self.id = id
        self.name = name


class Stats:
    def __init__(self, battles=0, frags=0, damage_dealt=0, wins=0, hidden=False, code=404):
        self.hidden = hidden
        self.battles = battles
        self.frags = float(frags)
        self.damage = float(damage_dealt)
        self.wins = wins

    @property
    def avgFrags(self):
        if self.battles == 0:
            return 0
        return round(self.frags / self.battles, 2)

    @property
    def avgDamage(self):
        if self.battles == 0:
            return 0
        return round(self.damage / self.battles, 2)

    @property
    def avgWins(self):
        if self.battles == 0:
            return 0
        return round(float(self.wins / self.battles), 4)*100


class Config:
    def __init__(self, serverId=0, region="eu", language="en"):
        self.serverId = serverId
        self.region = region
        self.language = language


class ReturnVal(Enum):
    SUCCESS = 0
    FAILED = 1
    DOUBLE = 2


class ErrorType(Enum):
    """
    UNKNOWN_PLAYER = 0
    UNKNOWN_SHIP = 1
    UNKNOWN_STATS = 2
    HIDDEN_STATS = 3
    UNKNOWN_SEASON = 4
    SERVER_ERROR = 5
    INTERNAL_ERROR = 6
    """
    def __str__(self):
        return str(self.value)
    UNKNOWN_PLAYER = 0
    UNKNOWN_SHIP = 1
    UNKNOWN_STATS = 2
    HIDDEN_STATS = 3
    UNKNOWN_SEASON = 4
    SERVER_ERROR = 5
    INTERNAL_ERROR = 6

these are my model classes, since I'm coming from C# I'm not too sure if this would be an acceptable way of dealing with it in python.

data/dbcontext.py:

####### Imports #######
import mysql.connector

####### Data imports #######
import data.log as log
from data.classes import Config, ReturnVal, Ship, ErrorType
from data.secret import Secret


def connect():
    ''' Returns the MySQL connection '''
    mydb = mysql.connector.connect(
        host=Secret.dbAddr, user=Secret.dbUser, passwd=Secret.dbPwd, database=Secret.dbName)
    return mydb


def getShip(name):
    '''
    Returns ship() the Ship ID, Name and URL from the Database, including Alternate Ship Names,
    Returns ReturnVal enum on error
    '''
    try:
        con = connect()
        cursor = con.cursor()
        sql = 'SELECT id,Name,url FROM Ships WHERE name LIKE %s'
        val = (name,)
        cursor.execute(sql, val)
        rows = cursor.fetchone()
        if rows is None:
            sql = 'SELECT id,Name,url FROM Ships WHERE id = (SELECT id FROM Asn WHERE name LIKE %s)'
            val = (name,)
            cursor.execute(sql, val)
            rows = cursor.fetchone()
            if rows is None:
                return ErrorType.UNKNOWN_SHIP
            else:
                return Ship(id=rows[0], name=rows[1], url=rows[2])
        else:
            return Ship(id=rows[0], name=rows[1], url=rows[2])
    except Exception as e:
        log.writeLog("getShip({name})".format(name=name), str(e))
        con.rollback()
        return ErrorType.InternalError
    finally:
        con.commit()
        con.close()


def addAsn(name, asn):
    '''
    Adds an Alternate shipname to the Database.
    Returns ReturnVal enum
    '''
    try:
        con = connect()
        cursor = con.cursor()
        sql = 'SELECT id,Name,url FROM Ships WHERE name LIKE %s'
        val = (name,)
        cursor.execute(sql, val)
        rows = cursor.fetchone()
        if rows is None:
            return ReturnVal.FAILED
        else:
            sql = 'INSERT INTO Asn (name,id,url) VALUES(%s,%s,%s)'
            val = (asn, rows[0], rows[2])
            cursor.execute(sql, val)
            return ReturnVal.SUCCESS
    except mysql.connector.IntegrityError as e:
        return ReturnVal.DOUBLE
    except Exception as e:
        log.writeLog("getShip({name},{asn})".format(
            name=name, asn=asn), str(e))
        con.rollback()
        return ReturnVal.FAILED
    finally:
        con.commit()
        con.close()


def getConfig(id):
    '''
    Gets the Server configuration from the Database.
    Returns Config()
    '''
    try:
        con = connect()
        cursor = con.cursor()
        sql = 'SELECT region,language FROM Config WHERE ServerId = %s'
        val = (id,)
        cursor.execute(sql, val)
        rows = cursor.fetchone()
        if rows is None:
            config = Config(serverId=id)
            sql = 'INSERT INTO Config(ServerID,region,language) VALUES(%s,%s,%s)'
            val = (config.serverId, config.region, config.language)
            cursor.execute(sql, val)
            return config
        else:
            return Config(serverId=id, region=rows[0], language=rows[1])
    except Exception as e:
        log.writeLog("getConfig({id})".format(id=id), str(e))
        con.rollback()
    finally:
        con.commit()
        con.close()


def addConfig(id):
    '''
    Adds an configuration of a new Server to the Database.
    Returns ReturnVal enum
    '''
    try:
        con = connect()
        cursor = con.cursor()
        sql = 'INSERT INTO Config (ServerId, region, language) VALUES(%s,%s,%s)'
        val = (id, "eu", "en")
        cursor.execute(sql, val)
        return ReturnVal.SUCCESS
    except Exception as e:
        log.writeLog("addConfig({id})".format(id=id), str(e))
        con.rollback()
        return ReturnVal.FAILED
    finally:
        con.commit()
        con.close()


def updateConfig(config):
    '''
    Updates an existing configuration of a Server in the Database.
    Returns ReturnVal enum
    '''
    try:
        con = connect()
        cursor = con.cursor()
        sql = 'UPDATE Config SET region = %s, language = %s WHERE ServerID = %s'
        val = (config.region, config.language, config.serverId)
        cursor.execute(sql, val)
        return ReturnVal.SUCCESS
    except Exception as e:
        log.writeLog("updateConfig({config})".format(
            config=str(config)), str(e))
        con.rollback()
        return ReturnVal.FAILED
    finally:
        con.commit()
        con.close()

In this file I'm handling all database access. The database stores mainly Ship names & IDs. Is this way of handling the database connection secure or is this vulnerable?

data/getStats.py:

####### Imports #######
import requests
import json

####### Data imports #######
from data.classes import Ship, Player, Stats, ErrorType
from data.secret import Secret
from data.api import api
from data import log


def getPlayer(config, playerName):
    '''
    Gets player Object from the Wargaming API
    Returns player() on success
    Returns ErrorType enum on failure
    '''
    try:
        url = api.psearch.format(
            reg=config.region, wgapi=Secret.api, playerName=playerName)
        response = requests.get(url)
        response = response.json()
        if response["status"] == "ok":
            if response["meta"]["count"] == 0:
                return ErrorType.UNKNOWN_PLAYER
            else:
                nick = response["data"][0]["nickname"]
                pid = response["data"][0]["account_id"]
                newPlayer = Player(name=nick, id=pid)
                return newPlayer
        else:
            return ErrorType.SERVER_ERROR
    except Exception as e:
        log.writeLog("getPlayer(config,{playerName}".format(
            playerName=playerName), str(e))
        return ErrorType.INTERNAL_ERROR


def getPlayerLink(config, player):
    '''
    Creates the Playerlink for the Wargaming Webprofile of the player.
    returns str() 
    '''
    link = str.format(
        "{}{}-{}", str(api.plink).format(reg=config.region), player.id, player.name)
    return link


def getStats(config, player, ship=None):
    '''
    Gets the players (optional ship) stats form the WargamingAPI
    Returns stats() on success
    Returns ErrorType enum on failure
    '''
    try:
        if ship is not None:
            url = api.sstats.format(
                reg=config.region, wgapi=Secret.api, accountID=player.id, shipID=ship.id)
        else:
            url = api.pstats.format(
                reg=config.region, wgapi=Secret.api, accountID=player.id)
        response = requests.get(url)
        response = response.json()
        if(response["status"] == "ok"):
            if bool(response["meta"]["hidden"]):
                return ErrorType.HIDDEN_STATS
            elif bool(response["data"]) and not response["data"][str(player.id)] == None:
                if 'statistics' in response["data"][str(player.id)]:
                    statistics = response["data"][str(player.id)]['statistics']
                else:
                    statistics = response["data"][str(player.id)][0]
                battles = statistics['pvp']['battles']
                wins = statistics['pvp']['wins']
                frags = statistics['pvp']['frags']
                damage_dealt = statistics['pvp']['damage_dealt']
                return Stats(battles, frags, damage_dealt, wins, False)
            else:
                return ErrorType.UNKNOWN_STATS
        else:
            return ErrorType.SERVER_ERROR
    except Exception as e:
        log.writeLog("getStats(config,{player},{ship})".format(
            player=player, ship=ship), str(e))
        return ErrorType.INTERNAL_ERROR


def getRankedStats(config, player, season, ship=None):
    '''
    Gets the players ranked stats for specified player, (optional) ship and season from the Wargaming API
    Returns stats() on success.
    Returns ErrorType enum on failure
    '''
    try:
        if ship is not None:
            url = api.rsstats.format(
                reg=config.region, wgapi=Secret.api, accountID=player.id, shipID=ship.id)
        else:
            url = api.rpstats.format(
                reg=config.region, wgapi=Secret.api, accountID=player.id)
        response = requests.get(url)
        response = response.json()
        if(response["status"] == "ok"):
            if bool(response["meta"]["hidden"]):
                return ErrorType.HIDDEN_STATS
            elif bool(response["data"]) and not response["data"][str(player.id)] == None:
                if ship is not None:
                    seasons = response["data"][str(player.id)][0]["seasons"]
                    if season in seasons:
                        currentSeason = response["data"][str(
                            player.id)][0]["seasons"][season]
                    else:
                        return ErrorType.UNKNOWN_STATS
                else:
                    seasons = response["data"][str(player.id)]["seasons"]
                    if season in seasons:
                        currentSeason = response["data"][str(
                            player.id)]["seasons"][season]
                    else:
                        return ErrorType.UNKNOWN_STATS
                stats = Stats()
                rankeds = []
                rankeds.append(currentSeason["rank_solo"])
                rankeds.append(currentSeason["rank_div2"])
                rankeds.append(currentSeason["rank_div3"])
                for ranked in rankeds:
                    if ranked is not None:
                        stats.wins += ranked["wins"]
                        stats.damage += ranked["damage_dealt"]
                        stats.battles += ranked["battles"]
                        stats.frags += ranked["frags"]
            else:
                return ErrorType.UNKNOWN_PLAYER
            return stats
        else:
            return ErrorType.SERVER_ERROR
        pass
    except Exception as e:
        log.writeLog("getRankedStats(config,{player},{season},{ship})".format(
            player=player, season=season, ship=ship), str(e))
        return ErrorType.INTERNAL_ERROR

this is the communication class for querying the API endpoints. The endpoints are stored in a different file data/api.py containing a class with the variables stored. i.e.

class api:
    psearch = "https://api.worldofwarships.{reg}/wows/account/list/?application_id={wgapi}&search={playerName}"
    pstats = "https://api.worldofwarships.{reg}/wows/account/info/?application_id={wgapi}&account_id={accountID}&fields=statistics.pvp.battles%2Cstatistics.pvp.damage_dealt%2C+statistics.pvp.frags%2Cstatistics.pvp.wins"
    sstats = "https://api.worldofwarships.{reg}/wows/ships/stats/?application_id={wgapi}&account_id={accountID}&ship_id={shipID}&fields=pvp.battles%2C+pvp.damage_dealt%2C+pvp.frags%2C+pvp.wins"
    plink = "https://worldofwarships.{reg}/community/accounts/"
    rpstats = "https://api.worldofwarships.{reg}/wows/seasons/accountinfo/?application_id={wgapi}&account_id={accountID}"
    rsstats = "https://api.worldofwarships.{reg}/wows/seasons/shipstats/?application_id={wgapi}&account_id={accountID}&ship_id={shipID}"

The code as shown here is currently working and being hosted on a linux server without too many troubles yet but I'm curious to know about the, probably many things, i could improve. If needed I can provide the answers I'm getting from the API if that would be of any concern to the code I've posted.

In general I'm seeking assistance to improve my python programming aswell as improve performance / stability of my code. Any information aswell as critique is welcome.

Thank you.

EDIT: Updated classes to include already done improvements.

data/secret.py:

class Secret:
    api = ""
    token = ""
    dbUser = ""
    dbPwd = ""
    dbName = ""
    dbAddr = ""

data/translations.py:

class translation():
    title = ""
    description = ""
    general = ""
    battles = ""
    avgDamage = ""
    winrate = ""
    footer = ""
    helpHeader = ""
    helpDescription = ""
    helpPlayer = ""
    helpShip = ""
    helpRanked = ""
    helpSRanked = ""
    #### UNKNOWN_PLAYER,UNKNOWN_SHIP,UNKNOWN_STATS,HIDDEN_STATS,UNKNOWN_SEASON,SERVER_ERROR,INTERNAL_ERROR
    error = []


class en(translation):
    title = "Statistics from {username}"
    description = "For {shipname}"
    general = "General stats"
    battles = "Battles"
    avgDamage = "Avg. Damage"
    winrate = "Winrate"
    footer = ""
    helpHeader = "Bot Usage"
    helpDescription = "How to use me correctly"
    helpPlayer = "Get the overall player stats"
    helpShip = "Get ship stats from the player"
    helpRanked = "Get season or sprint(e.g. 11 or s4) stats for a player"
    helpSRanked = "Get season or sprint(e.g. 11 or s4) stats for a ship from the player"
    #### UNKNOWN_PLAYER,UNKNOWN_SHIP,UNKNOWN_STATS,HIDDEN_STATS,UNKNOWN_SEASON,SERVER_ERROR,INTERNAL_ERROR
    error = ["Unknown Player", "Unknown Ship", "No stats recorded",
             "This player refuses to share his statistics", "Unknown Season", "Server Error", "Internal Error"]


class de(translation):
    title = "Statistiken von {username}"
    description = "Für {shipname}"
    general = "Allgemeine Statistik"
    battles = "Gefechte"
    avgDamage = "Durchschn. Schaden"
    winrate = "Winrate"
    footer = ""
    helpHeader = "Bot Benutzung"
    helpDescription = "So funktioniere ich"
    helpPlayer = "Für die allgemeinen Spieler Statistiken"
    helpShip = "Für die Statistkien des Spielers für das Schiff"
    helpRanked = "Für season oder sprint(z.B. 11 oder s4) statistiken eines Spielers"
    helpSRanked = "Für season oder sprint(z.B. 11 oder s4) statistiken des schiffs für den Spieler"
    #### UNKNOWN_PLAYER,UNKNOWN_SHIP,UNKNOWN_STATS,HIDDEN_STATS,UNKNOWN_SEASON,SERVER_ERROR,INTERNAL_ERROR
    error = ["Unbekannter spieler", "Unbekanntes Schiff", "Keine Statistiken vorhanden",
             "Dieser spieler möchte seine Statistiken nicht teilen", "Diese season ist nicht bekannt", "Server Error", "Internal Error"]


class pl(translation):
    title = "Statystyki gracza {username}"
    description = "dla okrętu {shipname}"
    general = "Ogólne statystyki"
    battles = "Bitew"
    avgDamage = "Średnie obrażenia"
    winrate = "Winrate"
    footer = ""
    helpHeader = "Bot Usage"
    helpDescription = "Jak używać bota poprawnie"
    helpPlayer = "Sprawdź ogólne statystyki gracza"
    helpShip = "Sprawdź statystyki konkretnego okrętu danego gracza"
    helpRanked = "Sprawdź statystyki sezonu bitew rankingowych lub sprintu dla gracza"
    helpSRanked = "Srawdź statystyki konkretnego okrętu wybranego gracza dla sezonu rankingowego/sprintu"
    #### UNKNOWN_PLAYER,UNKNOWN_SHIP,UNKNOWN_STATS,HIDDEN_STATS,UNKNOWN_SEASON,SERVER_ERROR,INTERNAL_ERROR
    error = ["Nieznany gracz", "nieznany okręt", "brak danych",
             "Gracz odmówił ujawniania swoich statystyk", "Nieznany sezon", "Server Error", "Internal Error"]


class tr(translation):
    title = "{username} oyuncusunun istatistikleri"
    description = "{shipname} için"
    general = "Genel istatistikler"
    battles = "Maçlar"
    avgDamage = "Ortalama Hasar"
    winrate = "Galibiyet Oranı"
    footer = ""
    helpHeader = "Bot Kullanımı"
    helpDescription = "Kullanım Kılavuzu"
    helpPlayer = "Oyuncunun bütün istatistiklerini al"
    helpShip = "Oyuncudan gemi istatistikleri al"
    helpRanked = "Bir oyuncunun belirli bir Sıralamalı Savaş veya Sprint(Örnek: 11 veya s4) sezonu istatistiklerini al"
    helpSRanked = " Bir oyuncunun herhangi bir gemide Sıralamalı Savaş veya Sprint(Örnek: 11 veya s4) sezon istatistiklerini al "
    #### UNKNOWN_PLAYER,UNKNOWN_SHIP,UNKNOWN_STATS,HIDDEN_STATS,UNKNOWN_SEASON,SERVER_ERROR,INTERNAL_ERROR
    error = ["Tanımlanamayan Oyuncu", "Tanımlanamayan Gemi", "Kayıtlı istatistik bulunamadı",
             "Bu oyuncu bilgilerini paylaşıma kapatmış", "Tanımlanamayan Sezon", "Server Error", "Internal Error"]

data/log.py

####### Imports #######
import os
from datetime import datetime

mainFolder = os.path.join(os.path.dirname(__file__), "../")
logFile = os.path.join(mainFolder, "bot.log")


def writeLog(method, message):
    '''
    Writes error messages to the `bot.log` file
    returns void 
    '''
    file = open(logFile, "a")
    file.write("{} | {} | {} \n".format(
        str(datetime.now().strftime('%Y-%m-%d %H:%M:%S')), method, message))

with this edit all needed files should be given for the bot to be able to run.

link for Wargaming API key

I've used the following script to create the database;

USE WOWS;


CREATE TABLE IF NOT EXISTS Ships(
Name NVARCHAR(255) UNIQUE,
Id BIGINT,
Url TEXT
);

CREATE TABLE IF NOT EXISTS Config(
ServerId BIGINT UNIQUE,
region NVARCHAR(255),
language NVARCHAR(255)
);

CREATE TABLE IF NOT EXISTS Asn(
Name NVARCHAR(255) UNIQUE,
Id BIGINT,
Url TEXT
);

Function to populate the database data/updateDB.py

# -*- encoding: utf-8 -*-
import codecs
import json
import os
import mysql.connector
import sys
import time


import requests

from classes import Ship
from secret import Secret

#   Vars
path = os.path.dirname(__file__)
apikey = Secret.api


#   Functions
def GetShipsByType(n, s, lang):
    response = requests.get(
        "https://api.worldofwarships.eu/wows/encyclopedia/ships/?application_id={id}&type={type}&fields=ship_id%2Cname%2Cimages&language={lang}&nation={nation}".format(type=s, nation=n, id=apikey, lang=lang))
    if(response.status_code == 200):
        jArray = json.loads(response.content)
        for x in jArray["data"].items():
            if x[1]['name'] not in ships:
                ships.append(
                    Ship(name=x[1]['name'], id=x[1]['ship_id'], url=x[1]['images']['small']))
                sys.stdout.write(".")
                sys.stdout.flush()
    sys.stdout.write("\n")
    sys.stdout.flush()


def db_connect():
    mydb = mysql.connector.connect(
        host=Secret.dbAddr, user=Secret.dbUser, passwd=Secret.dbUser, database=Secret.dbName)
    return mydb


def AddShip(newShip):
    try:
        con = db_connect()
        cursor = con.cursor()
        try:
            sql = 'INSERT INTO Ships(id,name,url) VALUES (%s,%s,%s);'
            val = (newShip.id, newShip.name, newShip.url)
            cursor.execute(sql, val)
            con.commit()
        except Exception as e:
            print("Skipped " + newShip.name)
            pass
        con.close()
    except Exception as e:
        print(e)


def cooldown(seconds):
    sys.stdout.write("cooldown")
    for x in range(seconds):
        time.sleep(1)
        sys.stdout.write(".")
    sys.stdout.write("\n")
    sys.stdout.flush()


#   Main
tmp = {}
toolFolder = os.path.join(os.path.dirname(__file__))
dataFile = os.path.join(toolFolder, "UpdateDbData.json")
try:
    with codecs.open(dataFile, encoding="utf-8", mode="r") as f:
        tmp = json.load(f, encoding="utf-8")
except:
    print("error")
    sys.exit(1)

langs = tmp["langs"]
nation = tmp["nations"]
shipTpe = tmp["types"]
ships = []

for l in langs:
    print("Getting Ships in Language: "+l)
    for n in nation:
        for s in shipTpe:
            print("Getting {} from {} ".format(s, n))
            GetShipsByType(n, s, l)
        cooldown(2)
    cooldown(3)
for s in ships:
    print(u"Adding {} to Database".format(s.name))
    AddShip(s)

Configuration for updateDB data/updateDbData.json:

{
    "nations": [
        "ussr",
        "japan",
        "germany",
        "france",
        "usa",
        "pan_asia",
        "pan_america",
        "italy",
        "uk",
        "commonwealth",
        "poland"
    ],
    "langs": [
        "en"
    ],
    "types": [
        "AirCarrier",
        "Battleship",
        "Cruiser",
        "Destroyer"
    ]
}
\$\endgroup\$
  • \$\begingroup\$ I cannot get your code to run - you're missing files data/log.py, data/secret.py, data/translations.py and api file statements api.plink, api.pstats, api.sstats, api.rsstats as well as a database structure. Or do you just want advice on what you've posted thus far? \$\endgroup\$ – C. Harley Apr 7 at 14:06
  • \$\begingroup\$ Update: I Updated and included all files needed to run this project. Sorry didn't thought about that running the project might be interesting aswell \$\endgroup\$ – Vulpex Apr 7 at 14:25
  • \$\begingroup\$ @C.Harley Okay I should've included everything now, wanted to reduce the length of the question but it makes sense to be able to run the project. The Project has been updated by after I posted it here hence I changed everything posted in the question. I'm looking for general do not dos aswell as hints for improvements. Just a general Idea how okayish the projects state is and what would need improvement and what would need dire improvement. \$\endgroup\$ – Vulpex Apr 7 at 14:39
3
+50
\$\begingroup\$

Thanks for the updates. First things first, when writing in a new language, it's best to learn what the standard for code formatting is, and Python has PEP8 and snake_case. I recommend learning them and conforming to that standard.

log.py - there's a standard logging package that comes with Python which can be used instead of rolling your own - and in main.py I see you import and instantiate it, but you use your own logger. Recommend spending a few mins reading the logging help pages.

At the top of each file you write ## imports ... ## (or a derivative of) - you don't need to make a comment for what your code already states. Only use comments to explain why, not how - that's for the code to explain.

As for the imports, the method you use should be the newer Python3 style - from package import class (or method). You can take advantage of relative imports as well, which are imports from the files point of view. For instance, in dbcontext.py you have from data.classes import Config, ... in my IDE, I change this into from .classes import Config, .... Have a look at the help pages about relative imports, and understand where you're executing your code from, everything should import from that dir tree.

main.py most of the code there looks discord specific but as mentioned, update the logging to use Python's logging package. You're missing an entry point if __name__ == "__main__": - always use this as it helps to point out where your code execution begins, and if you use packages that utilise reflection on your code, it will start running it instead of creating the objects for reflection.

api.py You create a class for what is a data structure. Here's a good tutorial on when to use classes - https://www.youtube.com/watch?v=o9pEzgHorH0 "Stop Writing Classes", quite an amusing watch and can improve your coding skills. Most likely the API only needs a dictionary for these definitions, and you can create that inside main.py (or going all the way, extract them out into an .ini and use ConfigParser - for proper separation and conform to the Open/Close Principle).

secret.py same as api.py
translations.py I'm not sure about this static inheritance overwriting the variables, I'm sure there's a cleaner way to do this, but shrug okay. Simple is fine too.

classes.py If you're using an if which will return either of two states, use a ternary. Also, 0 is nothing/false in Python - you can use a simple if check. To give an example of both suggestions:

@property
def avgFrags(self):
    if self.battles == 0:
        return 0
    return round(self.frags / self.battles, 2)

becomes

@property
def average_frags(self):
    return round(self.frags / self.battles, 2) if self.battles else 0

You can see that I renamed 'AvgFrags' to 'average_frags' - remember to name your variables/classes/method fully to make the intention clear. I'm not suggesting you go verbose - something like 'calculate_the_average_frags' would be excessive (I'm sure you know I what I mean).

dbcontext.py The first function is connecting to the database, which is an expensive procedure in terms of time, especially when you're doing it hundreds of times. You should move the instantiation of the db object into your __main__ and pass that when necessary into functions. Your connection string is verbose too - use unpacking to make this simple. For example:

def connect():
    ''' Returns the MySQL connection '''
    mydb = mysql.connector.connect(
        host=Secret.dbAddr, user=Secret.dbUser, passwd=Secret.dbPwd, database=Secret.dbName)
    return mydb

(you also don't need a comment, it's obvious what it's doing) becomes (when your kwargs are named the same):

def connect():
    return mysql.connector.connect(**Secret)

Further down, you're using fetchone() which is expensive too - the database will hold all the results in memory and maintain a thread with your script whilst you step through each answer. When you're writing to a database, if the dbadmin hasn't configured row locking properly (perhaps the db doesn't support it) - your loop can block other scripts from updating the table, a real traffic jam begins to pile up.
Recommend to use fetchall() and process the rows into a list of dictionaries. See here for some code: Identifying TCP and UDP streams using a database populated from a sniffer tool
Finally - all the functions in dbcontext really do look the same (DRY: Don't Repeat Yourself). It might be worth writing a basic select() and update() function which accepts (sql, values) and executes those so you remove the duplicated code from every function in this package file.

getstats.py I notice in the functions that the start is the same again (DRY) - it gets the URL for a type of stats, with the authentication data and some specific IDs - this is more DRY which we can replace.
We could create a helper which would handle the connection, authentication and query process, returning the result. It would be something like query_api(**kwargs) - where your function would unpack the named keyword arguments to determine what values are appropriate - like you have here:

if ship is not None:
    url = api.sstats.format(
        reg=config.region, wgapi=Secret.api, accountID=player.id, shipID=ship.id)
else:
    url = api.pstats.format(
        reg=config.region, wgapi=Secret.api, accountID=player.id)

It would handle error responses and logging, and return the HTML response code (200,404, etc) to the caller along with any data which is necessary (such as your statistics function). Something like:

return status_code, meta_data

updatedb.py Your GetShipsByType function has a magic string in it (the URL). This violates the Open/Close Principle (from the SOLID Principles), which states code should be open for extension but closed for modification - if your URL changes, you'd need to go back into the code to edit the URL before you can make it work again.
If you accidentally made an error, or unintentionally removed a line of code - do you have a backup or a git commit you can roll back? It's best to keep URLs and other magic numbers/strings outside the code, preferably in an .ini file.

To pick on the function a little more - you have some code which writes a dot to the screen when a new ship is being loaded. The Single Responsibility Principle states a function should do one, and only one thing.
If there's a problem writing that dot to the console (perhaps you're running a detached graphical session where there is no frame buffer/GDI - I've had that experience before) and it crashes the entire script?
I'm being a little dramatic with my point, but I'm sure you get what I mean. Hunting down bugs is much easier when a single function with 4 or 5 lines of code fails - then you only have to fix a single line (and not a chunk of code around the bug) and run your local tests again before committing/pushing into UAT.

You have a comment # Main - which implies you're running updateDB.py - remember to include the entry point if you are executing this package.

except:
    print("error")
    sys.exit(1)

When using Try/Catch - always catch the exception (run your code to see what exception it raises, and put that into the catch), that way other exceptions do not fail silently. Never let your code fail silently, even if it means adding another 10 lines to handle it. Either handle it or raise/throw it higher.
Using sys.exit(error_code) is only needed if your software which is executing this script is checking the return level/errorlevel status. If you don't need it, log the error and raise a RuntimeError("Could not load data file correctly") instead of exiting silently, forcing the operator to scratch their head and mabye run echo $? or echo %ERRORLEVEL% to find out the status code, then look into your code for what that status code means?

That's about all I can come up with this short review - I hope this was helpful?
Good Luck and keep up the coding!

\$\endgroup\$
  • \$\begingroup\$ Thank you very much for the suggestions. The only reason I exported the secret.py is that I actually have provided an secret_template.py, this is done to avoid commiting the api keys etc to git. What would be a common solution to avoid posting passwords and API keys to git, I'm planing to publish this project on git once I feel like it's "ready" so people can host it themselfs. Have a .ini file where they're written along with the API etc.? \$\endgroup\$ – Vulpex Apr 7 at 19:12
  • \$\begingroup\$ In my experience we usually have something like secret_template.py and a comment stating "copy this to secret.py to make it work" - with a .gitignore entry so secret.py is not committed/pushed. \$\endgroup\$ – C. Harley Apr 8 at 2:40
  • \$\begingroup\$ Okay so exactly the way I'm handling it at the moment. \$\endgroup\$ – Vulpex Apr 8 at 7:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.