Toma400 / The_Isle_of_Ansur

Python-based text RPG game, successor of Between Shadows and Light.
Other
9 stars 0 forks source link

Improving `get` handlers and `agnosticID` behaviour #84

Open Toma400 opened 1 year ago

Toma400 commented 1 year ago

While doing work on 0c97029, I realised that some systems are not properly coded, despite them running fine after some tweaking on my side. And so, even if this makes no big deal right now, when issues is closed with commit above, there should be further look into two sections & all functions using those:

  1. Getters Getters in various classes, profession.py Class, race.py Race and so on tend to have no way to check whether key is or is not in their system. While this passes the responsibility explicitly to caller function (such as here), this also obstructs important cases where it would be important to know whether this specific key is missing. This may be me just not sure on current design, because obviously we don't want caller to decide on KeyErrors and return None (it is much more annoying to handle exceptions, as we do it twice for issues made on None type as addition). Putting it as caller reponsibility seems purposeful, as if we don't expect KeyErrors, we just don't do try/except, or we can put log entry into it and reraise. It's probably issue part which should be just discussed whether there are better alternatives or if it's done the best way we could. Also checking uses of each getter would be nice, so we can put try/except clauses wherever it is needed.

  2. AgnosticID While getAttributesTupleAdjusted in attributes.py uses system that converts vague IDs into absolute ones, getAttribute (used so far only by function above, without issues) uses agnosticID converter, which seems to me to be totally out of place - the sole existence of agnosticID function was to pass vague IDs into absolute ones, yet somehow I passed this function to do exactly reverse work.
    So the question here is to advocate agnosticID against absoluteID converters and either find use for the former, or to unify them into absoluteID functionality. As I have really no idea what I wanted from agnosticID when I created it, or if I just were confused.