search

togfoxy / MarsLander

A re-creation of the 1979 Atari classic Lunar Lander with some new game play.
MIT License
5 stars 2 forks source link

Code reorganization Mega Issue #150

Open MadByteDE opened 2 years ago

MadByteDE commented 2 years ago

Overview

I think the code needs major reorganization to be able to add new functionality easier and keep the code readable for everyone. Having logic for different parts of the game in the main.lua or having one file containing all draw functions to me doesn't feel like a future proof concept.

I"m a OOP guy myself, but having separate files for stuff like different kinds of objects or categories like hud, menus etc. doesn't mean we have to follow the OOP paradigm with classes, inheritance, etc. It's just a different way to store the code to keep stuff together that is closely related.

I think this issue could be used to discuss further changes before working on it and going into a direction not everyone is cool with.

PRs

togfoxy commented 2 years ago

Every suggestion you've had so far is a winner so keep going. :)

MadByteDE commented 2 years ago

I made a new post in the love forums regarding a assetloader-tool.

Assetloader-tool.love

I've no idea why I've chosen to tackle this first, but I started cleaning-up some code and ended up with coding something to load assets :sweat_smile: .

What I actually tried to do is find a good directory structure to keep things organized. I think using the file names in the directory directly as name for the asset in the code makes sense to save some tedious work and avoiding having different names for each. We'd just need to make sure that we stick to one naming convention for the files to keep everything clean.

What do you think?

togfoxy commented 2 years ago

I like the idea but won't be in front of a computer for a day or too (I have access to mobile).

I assume this will help with frame rate, speed and memory efficiency?

MadByteDE commented 2 years ago

I like the idea but won't be in front of a computer for a day or too (I have access to mobile).

I assume this will help with frame rate, speed and memory efficiency?

In it's current form I'd say no, except for it to providing an easy way to create fonts and use them everywhere else, instead of creating a new font in the draw functions which is not really nessecary and potentially is bad for memory consumption and performance.

It also can be used to load directories of files one at a time e.g to just load menu assets first (ingame assets not needed yet) and when the player starts a new game the directory with in-game assets can be loaded. But it's not really meant to be used like this atm.

It's currently just to make our lifes easier and keep the code a bit cleaner. No fancy multi-threaded asset loading or anything like that. You'll know whenever you have time to look at the code. 👍

btw today I'll be back at work, so progress on my end will likely be a bit slower.

togfoxy commented 2 years ago

Okay. I took a look. I see what you're doing now. I like it. :) I thought this might be a just-in-time asset loader/unloader so that's where my questioning was but this is good. I like the way you 'buried' the anim8 stuff so the coder (us) doesn't need to know.

How do we progress? Is Assetloader.lua read as is? I'm happy for you to do a PR into main branch and we can all transition to it over time. It can make a large refactor a little more tolerable.

MadByteDE commented 2 years ago

How do we progress? Is Assetloader.lua read as is? I'm happy for you to do a PR into main branch and we can all transition to it over time. It can make a large refactor a little more tolerable.

Im currently refactoring lander.lua and also made some adjustments to the file directory to get ready for implementing the assetloader. But I've also already seen that there are some tricky things to resolve before being able to implement it, e.g the ids used for selecting which basetype is spawned and more.

I'll likely sent a draft PR today after work for you to be able to see what has been changed already.

togfoxy commented 2 years ago

On a different topic - I just fixed some stuff with the sockets and committed that to main branch with no conflicts.

Tomorrow I will refactor the same socket stuff (socketstuff.lua) and apply our style guide and see how we go. I look forward to nice clean and consistent code. :)

MadByteDE commented 2 years ago

I look forward to nice clean and consistent code. :)

Me too. I believe it might even be faster to rewrite large portions of the code rather then refactoring it, but from an educational standpoint I'd say refactoring is a better idea.

MadByteDE commented 2 years ago

Ok, I'd like to share some thoughts I have - mostly on how the source code refactoring is going imo.

But first of all I want to say - Even though it felt like all of us were pretty new with using git and Github in it's full potential, we managed to get used to our own little workflow almost in an instant (with some hiccups :roll_eyes: :smile: ). It's great fun to work on the project with you guys, especially because for me it feels like it's the first time such a collaboration just works "out of the box". Everyone seems to have their own small or big priorities and can work almost freely, everyone reviews PRs and gives feedback as much as the time allows - _& we even got something like an unintentional 24 hour shift because when you guys sleep I'm mostly doing something (like writing stuff like this :see_noevil:) - & vise versa :stuck_out_tongue_closed_eyes:

Okay, but now back to the project. I'd say the situation with the code got way more manageable. We got many objects and functions separated out to their own files, we added luacheck to our workflow and Github Actions which helps to find nasty stuff, we cleaned up most of the code and in our style guide, we defined how we want to write the code in future to keep it nice and shiny :star2: - all this in just a little over a week!

That said, I see more opportunities for improvements to some files. Here are some things I have in mind:

functions.lua:

createobjects.lua

enum.lua

main.lua If we decide we can use a new file for gamestate things, we could move many snippets handling the game's state like

    if strCurrentScreen == "MainMenu" then
        menus.DrawMainMenu()
    end

    if strCurrentScreen == "World" then
        drawWorld()
    end

from main.lua to the new gamestate.lua as well.

That should be pretty much all I can think of at the moment. :+1: Can't wait to see how the game will look in a couple more weeks :wink:

& in retrospect I'd like to say: how did this post get sOoo long?! Sry!

togfoxy commented 2 years ago

Thanks for the post and thanks for your contribution so far. @Doc and @Philbywhizz have of course contributed in ways that are just great. I think soon we'll settle into our own corner of the project (enet for me) and the PR's will become less painful. But I think we're getting better at the PR's.

I'll do this as a part of the enet upgrades and will make all that stuff isolated and immune to PR's (hopefully). Happy with the other suggestions but is it really possible to not have a 'generic' functions module that has things that are simply generic? We can try and see!

I'd love to get rid of createobjects.lua. That one is bugging me - lol.

All of this is great stuff and as we incrementally refactor it will only get better. :)

MadByteDE commented 2 years ago

Happy with the other suggestions but is it really possible to not have a 'generic' functions module that has things that are simply generic?

Sure, without any issues. We just need to get used to call modules locally in the files where those generic functions are used. That's the cleanest approach I can think of.

togfoxy commented 2 years ago

Noting that handle sockets isn't even called anymore. Feel free to completely delete it in your next PR.