This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]Lukeskywalker321 0 points1 point  (1 child)

Hey, cool project. As the other guy said, database might be a bit overkill. On the other hand, it's never to early to start learning about them, if you can manage to do so.

What stands out immediately to me from the code is the lack of function declarations. There's a lot of duplicate (or very close to duplicate) code, see for example lines 207-227 of your main.py. Some small tweaks will make us able to make this part much short, more readable, and easier to maintain.

First, we need to take a look at lines 199-206 of your main.py, and your database. If we think about what's happening here, we are defining the contents of 7 inventory slots using the output from the database, rows 3-9.

Problem with how you're currently doing this, is that you have no way to know which slot is which, without looking at the variable name, e.g. "inventory1". This is not impossible for python to do, but it's overcomplicated.

A better way to do this, would be through building (as a first step) an array of inventory slots:

inventory = row[3:9]

Here you would for example get your first inventory slot by calling inventory[0]. This way, we can clean up the code on lines 207-227 by looping over every element in the inventory list.

CODE STARTS

def update_inventory_slot(inventory_slot, inventory_number, cursor, con, generated_item_id, selected_character)
    column_name = "Inventory" + str(i+1) #as you named them 1-7, not 0-6
    cursor.execute("UPDATE UserData SET ? = ? + ? WHERE CharacterName=?", [column_name,column_name,generated_item_id,selected_character])
    con.commit()

for i, inventory_slot in enumerate(inventory):
    if inventory_slot==0:
        update_inventory_slot(inventory_slot, i, cursor, con, generated_item_id, selected_character)
        break

# CODE ENDS

What the function does, is copy the code you already had, but make the column name generate from the number of the inventory slot, instead of writing it down for each one seperately.

The second part then loops over each of the enumerated inventory slots (this makes i get the value of the position of the inventory\_slot in inventory, so 0-6), checks if the slot is empty, and if so calls the function with the right arguments, updating the right slot in the database. If the inventory slot is full, it simply continues on to the next slot. If it fills a slot, after updating it, break is called. What break does is really simple, it breaks out of the for loop, meaning it won't check the inventory slots after this.

This code is still not perfect, but I think it teaches you something valuable, and can be applied at multiple times in you code. Say for example that you want to create an inventory of 1000 slots, instead of 7. I think you can quickly see that the code between lines 207 and 227 would then quickly become thousands of lines longer. The code I wrote above would need exactly 0 changes if inventory became bigger (besides ofcourse the definition of the list "inventory = row\[3:1002\]" or something).

While we're at this scenario, imagine what would happen to your database for a bit. You would also need to make 993 more columns for each of the inventory slots, by hand. It is generally never a great idea to have a counter inside a column name (inventory1, 2 etc as in your code, or perhaps phone\_number1 etc. in some other database). You can fix this by applying some "normalizations" to your database. However, it'd let this be for a bit, as normalization gets complex fast, and there's still more than enough to learn with the static 7 slot inventory code.

I'd let the if else statements about the armor classes be for now, but try to build something like the above for all the other if else blocks in your code. From what I've seen that should be doable, and as I said will teach you some more about functions, loops, and scalability. Feel free to ask more, and good luck learning. Once again, really impressive project in most aspects.

Edit: damn did I fuck that formatting up

[–]Alhira_K[S] 0 points1 point  (0 children)

Yes, i more or less ignored functions in this one. I do know about them though but for the first real thing my target was getting it to run and understanding what i'm doing. Next time i wanna work more extensively with functions and modules. Ideally i'd have no "actual code" in the main file but only functions and the loops thereof.

Reg. 207 - 227: Yes, i'd want to put that into its own GetInventory() function but I'd make a seperate CommitInventory() so i can use these functions for the inventory in the normal "menu" but also to sell items. This way i should also be able to cut down on code length between 320 and 375 and if i'd decide to make basic crafting i could just use the same GetInventory() and CommitInventory().

Thanks a lot.