Remove the passing around of 'dwgfx', 'map', 'obj', 'music', 'key', and 'help' #188
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I've decided to call dwgfx/game/map/obj/key/help/music the "global args". Because they're essentially global variables that are being passed around in args. This commit removes global args from all functions on the Game class, and deals with updating the callsites of said functions accordingly. It also renames all usages of 'dwgfx' in Game.cpp to 'graphics', since the global variable is called 'graphics' now. Interesting to note, I was removing the class defines from Game.h, but it turns out that Graphics.h depends on the mapclass and entityclass defines from Game.h. And also Graphics.h spelled mapclass wrong (it forgot the "class") so I just decided to use that existing line instead. This is only temporary and after all is said and done, at the end of this pull request those class defines will be gone.
This commit removes all global args from functions on the entityclass object, and updates the callers of those functions in other files accordingly (most significantly, the game level files Finalclass.cpp, Labclass.cpp, Otherlevel.cpp, Spacestation2.cpp, WarpClass.cpp, due to them using createentity()), as well as renaming all instances of 'dwgfx' in Entity.cpp to 'graphics'.
This commit removes the global args being passed around from the function args on the mapclass object, as well as updating all callers in other files to not have those args. Furthermore, 'dwgfx' has been renamed to 'graphics' in Map.cpp.
This removes global args from all functions on the Graphics class. Callers of those functions in other files have been updated accordingly. Of course, since Graphics.cpp is already in the Graphics namespace, I do not need to change all 'dwgfx' to 'graphics' in Graphics.cpp.
This commit removes all global args from the parameters of each function on the scriptclass object, and updates all places they are called accordingly. It also changes all instances of 'dwgfx' to 'graphics' in Script.cpp. Interestingly enough, it looks like editor.h depended on Script.h's class define of the musicclass. I've temporarily placed the class define in editor.h, but by the end of this patchset it'll be gone.
This removes global arg passing from all functions on editorclass. Callers have been updated correspondingly. Additionally, all 'dwgfx' has been replaced with 'graphics' in editor.cpp.
This commit removes the passing around of global args in the logic functions. Additionally, all 'dwgfx' has been replaced with 'graphics' in Logic.cpp.
This removes all global args from functions in Input.cpp. Additionally, 'dwgfx' has been renamed to 'graphics' in Input.cpp.
This removes all global args in all functions in titlerender.cpp. Additionally, all 'dwgfx' has been renamed to 'graphics' in that file (there are a lot of them, as you might guess).
This removes all global args from preloader.cpp, and changes all 'dwgfx' to 'graphics'.
This removes global args from Finalclass.cpp, Labclass.cpp, Otherlevel.cpp, Spacestation2.cpp, and WarpClass.cpp.
The argument provided to entityclass::getcompanion() does absolutely nothing. Remove it, and update all callers.
The two arguments 'col' and 'col2', both the only integer arguments of the function, do absolutely nothing. Remove those and update callers.
The 'r', 'g', and 'b' arguments do absolutely nothing, even though they're used in the version of Graphics::drawtile() that's more used. So delete the other version without those extra arguments, and then remove the extra arguments from the remaining version. And then update callers.
The 'r', 'g', and 'b' arguments do absolutely nothing. Except unlike Graphics::drawtile(), there's only one version of Graphics::drawtile2(), so just remove those args and update callers.
This function is never used anywhere in the game.
This function does absolutely nothing. It is, however, called once in gamecompletelogic2(), making it technically used.
This variable does absolutely nothing, so I'm removing it.
Although it keeps getting set to true and false in various places, it never once gets checked, essentially deeming it a variable that's used but does nothing.
Even though it keeps getting set to true and false everywhere, it never once gets checked. So it's a variable that does nothing.
'swfStage' gets set to 'stage' in updategraphicsmode() but... that does absolutely nothing, because they both contain exactly the same thing. And these variables aren't referenced anywhere else. So I'm removing both of these variables.
Since I removed the useless variables 'stage' and 'swfStage' earlier, now this class is unused. So I'm removing it, too.
Now that it does nothing due to some earlier changes, it's a useless function that does nothing. (Well, it was already a useless function, but those earlier changes made it clearer just exactly how useless it is.) So remove that function and remove all its callsites.
This function does not clear the screen at all, it does absolutely nothing.
It's a function that does nothing, used by no one.
Apparently the 'offset' argument did something in the 1.x Flash versions, but now it does nothing.
Just to be in line with the removal of the argument in the actual function itself.
This changes something like UtilityClass::String to help.String, basically. It takes less typing this way, and is a neat effect of having global args actually be global variables.
Surprisingly, there's not a lot of it. There is, however, a lot of mixed indentation in this project.
This removes all indentation that suddenly switches in the middle of a function. Most particularly egregious offenses are the ones made by the person who has 2-wide tabs, but keeps tabbing up to make each indentation level match up with the 4-wide spaces, so to them (and only them) it will look just fine, but since by default tabstop is 8-wide, their lines are pushed off all the way to the right.
There is a lot to be had for this one.
There's a lot, looks like mainly because of the person who added MMMMMM and M&P.
Not that much, but only because it's not a big file to begin with.
Only 3 lines, which is really impressive and the lowest amount of mixed indentation so far.
And this is the winner for having the least amount of mixed indentation that is nonzero.
Also fix the indentation of preprocessor statements.
It looks like this may have been used earlier in development, judging from the name, obviously, but right now it seems like it's used as an error message if a main game level is asked for an invalid room (well, only two of them - the Lab and Warp Zone). It should probably be formalized into an error system, if we want to keep teststring, and also people would never see it anyway because I don't think there's a reliable and consistent way to trigger loading a non-existent room. I have seen someone manage to load a non-existent Warp Zone room only one time, but even then this teststring didn't pop up. So this teststring doesn't even trigger in the right circumstances. Also, when it does pop up, as far as I can tell it will stay onscreen, which is kinda annoying. So I'm just removing this ancient relic from the code.
Unlike, say, the scriptclass i/j/k stuff, these tempstrings are only purely visual, and there's no known glitches to manipulate them anyway. Thus I think it's safe to make this cleanup.
Most of this is telecookie/quickcookie stuff, which was used in the Flash version, but there's no longer any such thing as a save cookie. Also one TODO that says to make a function that's now been made.
This removes a bunch of commented-out code that was clearly kept from the Flash version, even though the Flash graphics API is much different than SDL's. Also removes a bunch of TODOs that either say nothing, or say something whose meaning has been totally lost to time due to being completely vague, or something that's already been done and someone forgot to remove the TODO.
Lots of these are from the Flash version!
A lot of these seem to be based on an earlier version of the C++ port, but they left some Flash stuff (like the buffer lock/unlocking) in, too.
Some of these seem to be copy-pasting stuff and then commenting it out if it doesn't fit the pasted case.
Surprisingly not that many. It looks like at one point in development, damage blocks were created for every single spike in the Tower, before it was too laggy so they switched to directly checking collision with the tile instead.
Earlier design decisions that have been commented out and are now obselete.
This is the "Behavioral logic", "Basic Physics", and "Collisions with walls" trio. They were originally aligned but then I removed global args, thus misaligning them. So now I'm re-aligning them back again.
There's a commented-out recording input option, which sounds interesting.
Lots of stuff from the Flash version, especially in Music.h.
It's a function that does nothing, and is also used by nothing.
Just a miscellaneous code cleanup. There's no glitches that take advantage of the previous situation, namely that 'temp' was a global variable in Logic.cpp and editor.cpp. Even if there were, it seems like it would easily lead to some undefined behavior. So it's good to clean this up.
Mostly comments from all the global args were not-so-global args, along with input recording.
Most of the code was already commented out, and those comments were removed in earlier commits, but this removes all recording variables from Game and simplifies the game-gamestate handling in main.cpp a little bit.
|
Beautiful, I’ll roll through everything in the morning. |
Was just skimming through my diffs, looks like I missed these comments in preloader.cpp.
|
Everything looks good, thanks for all the hard work on this! To contrast, I burned out on the previous version after a couple hours, this took about 10 minutes. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes:
Summary
This PR is a re-do of #178, except this time everything is not in one commit. It has been split up into 62 commits. Also it contains more cleanups that I had missed before.
dwgfx,map,obj,music,key, andhelpare no longer passed around as parameters in any function that needs them. Additionally, alldwgfxhas been renamed tographicssince it's a clearer name (I'm not sure what the "dw" in "dwgraphics" stands for, and it sounds like a remnant from the Flash days).Additionally, since this change affects pretty much the entire codebase, I also took it upon myself to clean up mixed indentation, remove outdated comments, remove useless or unused functions, remove unused parameters, remove unused stuff like testing code and input recording code, and remove the
tempglobal file-level variable and movetempstringoff of class objects. Also, all expressions of the formUtilityClass::somethinghave been changed tohelp.something.The standard of indentation I want is to at least have files be consistent with their indentation. A file can indent with tabs or spaces, but it ought to be at least consistent. The annoying part is when you suddenly start switching indentation in the middle of a function. The exception here is
Graphics.cpp, which switches to tabs starting fromGraphics::setcol(), which I thought was fine because it wasn't in the middle of a function, and it consistently uses tabs belowGraphics::setcol()and spaces above, so it's fine.The following functions had unused parameters:
entityclass::getcompanion()Graphics::Hitest()Graphics::drawtile()Graphics::drawtile2()musicclass::playef()The following functions or variables were unused or useless:
entityclass::checkdirectional()Game::telegotoship()Game::sfpsmodeGame::telecookieexistsGame::quickcookieexistsstageswfStageclass Stageupdategraphicsmode()Screen::ClearScreen()musicclass::initefchannels()Screen::glScreeneditorclass::weirdloadthing()(instead, make a copy of the string and useeditorclass::load())musicclass::processmusicfade()Legal Stuff:
By submitting this pull request, I confirm that...
example, a 2.3 update on Steam for Windows/macOS/Linux)
CONTRIBUTORSfile for all of said releases, butwill NOT be compensated for these changes