2021-01-04 23:58:35

Programmers everywhere warn to not use global variables. I try to design that way myself, but I'm coming into an issue where I think I'm reaching outside my field of knowledge - which is admittedly pretty shabby.

I'm making a controller utility in Python. Originally, the idea was to make something that will display battery level, since that's not accessible to us but is shown in the game bar and can be gotten programmatically. I've done that, using pygame as the window source.

I'm using a package called XInput-python. Right now, I'm only tracking connect, disconnect and button events, but I want to handle stick and triggers as well. The problem is that I want the tts to announce when you move the stick about halfway. It should say left stick or right stick. I haven't decided yet if I want to deal with left stick left, left stick up, etc. But it is definitely possible within that package. There should also be a threshold so that if you say, cross x -0.50000 then x -0.49999, it doesn't retrigger.

Since the events come in so rapidly, it just overwhelms the speech, which is not what I want. So I had the idea to run it through a function called handle_sticks() which would check the values, announce a message, but then set one of the elements in a tuple to True. There would be four. One for each stick, and one for each trigger.

Where I run into trouble is getting around this concept without using global variables. There needs to be a way to keep track of the state of this tuple between function calls. If I return the tuple, then pass it back into the function that creates it in the first place, it'll just get overwritten.

So how can I deal with this without using global variables. I'll paste the code as it stands right now, since I am not overly protective of it, and this can then be a way to gauge feedback from the things that I've done but could be done better, plus hopefully, an answer to my original question.

Click here to view the code, the password is bunnies.

Facts with Tom MacDonald, Adam Calhoun, and Dax
End racism
End division
Become united

2021-01-05 00:13:12

Don't have time to review the code but the answer to this sort of problem is almost always "make a class, then put the variables on the class".  When it gets big enough, figure out how to separate it into separate classes with different responsibilities,but it sounds like that won't be your problem for a while.

Also read-only global variables are called constants and are perfectly fine, so if you want to just hardcode some configuration into the program, sure.  Just comment them so you know what they change, and don't do it if you think they're going to come from a file later--or at least, not without some thought.

My Blog
Twitter: @ajhicks1992

2021-01-05 00:25:09 (edited by GrannyCheeseWheel 2021-01-05 00:31:26)

I was trying to avoid all that. I hate OOP constructs. It always just seems so over-engineered for the problem at hand. I guess I will have to at some point though.

I can't really use constants here because each variable or element in a tuple or list is supposed to be a flag. If the tts has announced the fact that the stick has moved, but the stick remains within the opening and closing range, nothing more will be announced.

Facts with Tom MacDonald, Adam Calhoun, and Dax
End racism
End division
Become united

2021-01-05 00:38:22

Also, a trick I've used when dealing with GPS coordinates, save a spoken position. Initialise it to None:

class StickClass:
    spoken_option: Optional[float] = None

    def stick_thing(self, position: float) -> None:
        if self.spoken_position is None or (max(position, self.spoken_position) - min(position, self.spoken_position)) > 0.2:
            self.spoken_position = position
            speak(f'New position is {position}.')

That makes sure the position has to change by 0.2 before it'll speak the difference.

Particularly useful when you've got some really sensitive data like GPS or your stick position, to not get spammed. Play with the value until you're happy.

-----
I have code on GitHub

2021-01-05 00:45:48

Classes aren't OOP unless you want them to be.  Sure, you can go do a bunch of stuff with inheritance and have your nice 6-level hierarchy and UML diagram if you want, but let's not.

The best way to think of classes is as a sort of bag where you can put variables, and functions that can see those variables.  Got some variables that need to live longer than a single function call? Indent the function, add self, wrap indented function in class, add __init__ to set them to initial values.  Got some other functions that need to see those variables and do something kinda related to the other function?  Stick in same class.  Done, just make an instance somewhere and go.

This isn't exactly the best way to think about it because this makes classes sound like they're modules, and you really do want to think about responsibility because some sort of giant class with 500 functions isn't going to do anyone any good, and yeah you should probably start thinking about classes as objects.  In general you're asking a question "how do I design my program better" while also saying "I want to ignore the thing that'd let me design my program better" which isn't exactly the most constructive.  But as a first approximation that will stop you from getting it completely wrong, the above isn't a bad way to think about it for now.

@chrisnorman7
For what it's worth putting variables at the class level rather than initializing them in __init__ may be slower by enough to matter with the stuff you do, but is mostly irrelevant to this thread.  It's been a long time since I've looked into it, but you may want to benchmark because my intuition says it's a 2x performance hit until the first time something writes the variable on the class because of how Python attribute lookup works.

My Blog
Twitter: @ajhicks1992

2021-01-05 02:15:41

I'm not saying I'm not gonna do it, In fact, I will. I am just grumbling about it because I hate classes. The second you start using them, then everything becomes a class and you've got objects all over the place because you need one for this and that and the other.

Facts with Tom MacDonald, Adam Calhoun, and Dax
End racism
End division
Become united

2021-01-05 02:35:57

Nah.  Synthizer is like 6000 or 7000 lines at this point and counting.  It's still under 50 classes despite using classes for lots of things.  Some free functions do algorithmic heavy lifting, but nonetheless.  Whoever taught you that classes breed like rabbits was wrong.  I'm assuming you looked at a Java tutorial at some point or something.

It is possible for classes to proliferate in something like a game because inheriting from one to define new enemy types or something is a pretty good way of doing that, but in that case you've got an abstract interface and only a very small part of your code deals with the specific classes.  There's just this generic thing called enemy everywhere else.

My Blog
Twitter: @ajhicks1992

2021-01-05 11:30:22 (edited by Dragonlee 2021-01-05 11:35:46)

global mutable state sure is something you want to avoid, since it can be accessed anywhere, making it difficult to reason about. one way to limit the scope definitely is what has been proposed, to use a class and make the mutable state a field of the class. that way only the methods should be the ones mutating that state, but I would actually agree with OP that OOP often leads to over engineered code and can actually get really messy. not saying OOP is totally bad, but it is also entirely unnecessary. yes it is necessary to use classes in python to create convenient packages of various variables grouped together, however it is not necessary to do the Oop thing of mixing data and logic by giving those classes methods that are responsible for operating on the fields of the class.

I can propose a simple procedural style, data-oriented

design where data and logic are seprated, which imo is simpler and cleaner.

Again, not saying this is the good way to do it and the Oop way is the bad way. The OOP way is fine too, but just want to provide an alternative, as this thread is giving the impression that it is the only way.

So how else can we limit scope of mutable state? Well simplest way is instead of global variables to have local variables, either in a loop or a function. Let’s say I have a main function that gets called when the program starts up. You can just initialize all your state as local variables and when you need some function to have access to it, you just explicitly pass it as a argument to that function. That function can then read that state you pass it and if it is mutable  then it can also change it. Small example below:

def main():
  state = True # in this case state is just a boolean
  while True:
    foo(state) # foo will be able to read state
    # since booleans like all primitives are immutable, in order for a function to change it
    # it will have to return a new value and the caller will have to update the state
    new_state = bar(state)
    state = new_state
  # end while
#end def

If you want to package related data together so it is more convenient to pass around and also gives you a mutable container then the simplest and most efficient is to use pythons relatively new dataclasses.

from dataclasses import dataclass

@dataclass
class InventoryItem:
    """Class for keeping track of an item in inventory."""
    name: str
    unit_price: float
    quantity_on_hand: int = 0

this neat decorator will auto generate an __init__ and __repr__ function, so you can use it simply like this:
item = InventoryItem("chocolate",  2.5)
item.unit_price #access a field
item.quantity_on_hand  = 50 # mutate the state

since the dataclass acts as a mutable container, you don't have to do as was done in my previous example of having the function return a new state and the caller being responsible for updating it. although that way has major advantages in being able to reason about the code and making your functions testable. instead when passing a dataclass, the function can mutate it directly:

def sell_item(item: InventoryItem):
  item.quantity_on_hand -= 1 # change is reflected everywhere

2021-01-05 11:59:57

@5
Oh wow, I'm screwed then. I make all my classes with the attrs package haha.

I love having the vision to create great things, but not the brains to make them truly awesome.

Thanks for the heads-up though, I'll look into it.

-----
I have code on GitHub

2021-01-05 16:07:37

@8
I entirely fail to see how what you are proposing is helpful for a new programmer.  There is a reason people don't start with functional programming.  You tend to provide entirely correct answers without considering your audience, which is something you should probably work on.

Also, your approach is almost exactly equivalent to classes anyway, at least in Python.  Except that you've added several layers of obfuscation and basically said "implement what classes do manually".

I'm not saying you're wrong necessarily, just that you really have to consider who you're talking to.  "Have you considered not using classes and adopting functional programming, only to end up back where you started except using a bunch of things you probably weren't ready for?" is essentially what you're saying right now.  OP isn't ready to finish going down the road of functional programming and probably doesn't have the experience to do so without ending up with spaghetti code.  While it is reasonable to say that maybe we could teach more functional programming at the beginning or something, that isn't the world we live in at the moment.

@9
It's probably fine enough for now, it's just something you should be aware of.  Also, attrs may deal with it, and it may not even be a problem depending on what benchmarks say.

My Blog
Twitter: @ajhicks1992

2021-01-05 18:09:31

@10
OK, thanks.

-----
I have code on GitHub

2021-01-05 18:14:14

@10 although I do often advocate functional programming, this instance I am not actually advocating FP. the alternative solution I am proposing is just regular procedural imperativ programming.

and I think in this case I am considering the audience, since my proposal is simpler than jumping to doing OOP, since it doesn't require OP to adopt any new paradigm and to start thinking of how to organize their code in classes. The OP specifically complained about needing to learn OOP in order to solve this problem and so I am providing a way that doesn’t require any OOP sort of design, but instead just use local variables as a way to not need to deal with global mutable state. I’ll just re-iterate I don’t have a problem with OOP, but in this case it is much more doing OOP for OOP sake and the problem can be tackled more simply in a strictly procedural non-OOP way.

2021-01-05 20:49:34

I think what I'll end up doing is making classes that you call directly without having to create objects for them. That's what seems to make the most sense in this particular case.

So by me putting the variables right under class rather than in the __init__(): block, they stay at class level and I can just call class methods to change things.

Facts with Tom MacDonald, Adam Calhoun, and Dax
End racism
End division
Become united

2021-01-05 22:30:24

@13
That feels like extra typing, and you don't get much that using a module doesn't already give you.

Why not instantiate the classes somewhere? A nice little application.py module should do it, then you can keep all your instantiated whatnots in there. That way, if for some reason (multiple controllers in your case maybe?) you can simply create a new instance.

-----
I have code on GitHub

2021-01-05 22:44:20

I should be able to handle multiple controllers as things stand right now, because each one gets its own index which I pass to functions that will use it.

I just don't see the point in all that. It seems silly to me to go through all that just to use one thing one time. The announce thing will only ever be used once. It's not a game where you make classes for weapons, then a subclass for melee weapons, a sibling for projectile, etc. That makes sense to me. You'll want multiple guns, multiple knives and swords, and the ability to spawn them at any time. But for the announce thing, IDK it just doesn't make sense to me to make a full on class then instantiate an object for something you only use once.

Facts with Tom MacDonald, Adam Calhoun, and Dax
End racism
End division
Become united

2021-01-06 03:30:20

If your program is too small to deserve not using globals, if you are going to finish this then never touch it again, then just use globals.  Doing class-level variables that get changed by classmethods doesn't help you because they're globals, you've just put a little bit of code on them so that you can pretend they're not and feel good about it.

I suspect what you want is a singleton.  To do that, make a class like you're going to make an instance, then make an instance of that class and use it as the global.  It's not as good as not having globals, but it's better than making everything a global.


There is a difference between a script and a program.  A script does one thing and is effectively write only.  You are talking about this like it's a script.  Scripts don't deserve being clean code.  You finish them, they're small, they're done forever, it doesn't matter.  If that's the case, wait for a bigger project in which to learn about modularity and avoidance of globals.

My Blog
Twitter: @ajhicks1992

2021-01-06 03:53:03

Yeah it's not a huge thing. I did start working on a class for the announce state. I decided to do it properly because I can pass in the controller index into the constructor so it will handle multiple controllers. But then I have to figure out a way to be able to pull the values off the event queue into the class to set the variables. I want to compare current vs. old to get a difference value.

Facts with Tom MacDonald, Adam Calhoun, and Dax
End racism
End division
Become united

2021-01-06 14:45:57

@17 it is starting to seem to me that the issue you are running into is more a matter of having your state be in scope in the function that handles the event loop and to update it between iterations of the loop. I cant access your code, but if you could show how the event queue code is structured then that would allow to give better advice

2021-01-06 18:31:26

3

import logging
import sys
import os


os.environ["PYGAME_HIDE_SUPPORT_PROMPT"] = "hide"

import pygame
import XInput as xin
from cytolk import tolk
from pygame.locals import *

logging.basicConfig(
    filename="controller_info.log",
    level=logging.DEBUG,
    format="%(asctime)s  %(levelname)s:\t%(message)s",
    datefmt="%m/%d/%Y  %I:%M%p",
)


def buzz(state):
    if state:
        xin.set_vibration(0, 0, 1.0)
    else:
        xin.set_vibration(0, 0, 0)


def setup_speech():
    logging.info("Initializing Tolk")
    tolk.load()
    tolk.try_sapi(True)
    sr = tolk.detect_screen_reader()
    if not sr:
        tolk.prefer_sapi(True)
        logging.warn("No screen reader detected, falling back to SAPI.")
    else:
        logging.info("Proceeding with %s", sr)


def get_battery(controller):
    battinfo = xin.get_battery_information(controller)
    if battinfo[0] == "DISCONNECTED":
        battstring = "there is no battery present"
    else:
        battstring = f"the battery is of type {battinfo[0]} and is operating at a {battinfo[1]} level of charge"
    return battstring


def main():
    logging.info("Initializing pygame")
    pygame.init()
    screen = pygame.display.set_mode((400, 200))
    pygame.display.set_caption("Controller Information")
    background = pygame.Surface(screen.get_size())
    background = background.convert()
    background.fill((250, 250, 250))
    clock = pygame.time.Clock()
    setup_speech()

    logging.info("Entering main loop")
    while True:
        for event in pygame.event.get():

            if (
                event.type == QUIT
                or event.type == pygame.KEYDOWN
                and event.key == pygame.K_ESCAPE
            ):
                tolk.speak("Exiting")
                pygame.time.delay(1000)
                logging.info("Exiting program")
                return

        for inevent in xin.get_events():
            if inevent.type == xin.EVENT_CONNECTED:
                tolk.speak(
                    f"Controller {inevent.user_index + 1} has been connected. {get_battery(inevent.user_index)}"
                )
            if inevent.type == xin.EVENT_DISCONNECTED:
                tolk.speak(
                    f"Controller {inevent.user_index + 1} has been disconnected."
                )
            if inevent.type == xin.EVENT_BUTTON_PRESSED and inevent.button == "A":
                buzz(True)
            elif inevent.type == xin.EVENT_BUTTON_RELEASED and inevent.button == "A":
                buzz(False)

        screen.blit(background, (0, 0))
        pygame.display.flip()
        clock.tick(60)


if __name__ == "__main__":
    main()
Facts with Tom MacDonald, Adam Calhoun, and Dax
End racism
End division
Become united

2021-01-06 18:54:57

@19
I tell you what, that is some nicely laid out code!! We always moan when code is badly formatted, or not formatted at all, but that is lovely to read. Well done.

-----
I have code on GitHub

2021-01-06 19:00:37

Dude it wasn't me, it was Black, and it's set to autoformat every time I save... lol.

Facts with Tom MacDonald, Adam Calhoun, and Dax
End racism
End division
Become united

2021-01-06 20:13:35

your code looks quite clean. for keeping track of state between iterations of the main loop, I think just setting up state inside main() before the main loop starts should be fine. then you can update the state inside the main loop and it will track it between the different iterations of the loop. when a subprocedure needs access to it you can pass it in as an argument like you are doing for buzz(). and if a subprocedure like buzz needs to be able to update the state then it can return a new one that will be updated in the main loop, something like:
state = buzz(state)

the thing this earns you over using global variables is that if you suspect there is some bug with how state is being updated, you know you just need to look inside the main loop code, since that is the only place that has access to changing it, whereas with a global variable you'd need to scan all the code.

2021-01-06 20:52:49

Gotcha, yeah I think I will go with this approach.

Facts with Tom MacDonald, Adam Calhoun, and Dax
End racism
End division
Become united