Should we be looking into modules ?

[Diyagi]

The script is getting more and more complex over time, thanks to you awesome work. With this in mind, i would like to suggest the use of lua modules, by using them, we can segregate the script into multiple modules, this would make it more easier for later developers to start working in this project.

I did a quick testing, and seems that the Reframework does support loading modules from a inner folder inside autorun, with this is possible to create a structure and keep things clean.

Of course this involves a quite big refactor, maybe i could help, even tough i dont like Lua that much (i had a very bad time coding for FiveM), so before i try anything, i want to see your opinion on this.

[coavins]

This is a great suggestion, I've been intending to look into whether this works with REFramework but I haven't had the chance to explore this yet.

It might be difficult to merge such a big refactor with my own heavy refactoring still going on, so I would advise caution on attempting any large rewrites of this nature at this time. It would be better to wait until I eventually stop changing such large portions of the code with every update.

With that said, there are probably some useful things we can learn about how this works. You could maybe start by pulling out one small, self-contained piece (such as the damage counter or source) into a separate module and see what kind of work is involved. It would also be important to find out whether reframework loads files from within subscripts inside the autorun directory.

[Diyagi]

I will start by moving some of the tables (like ENUM_KEYBOARD_KEY, ENUM_KEYBOARD_MODIFIERS, ATTACKER_TYPES, and others) to a separate file, do some more testings and see how things goes.

[coavins]

Sounds like a good place to start, I'm not a fan of those tables taking up so many lines.

I was trying to think of other things that might make good candidates, and I think the first major component to go after here would probably be the section responsible for drawing the report. This would be the three or four functions that draw the table text and damage bars. I think they should be isolated enough from the report generation that a clean break could be made. The only question is I'm not sure whether the globals exposed by reframework would be accessible from there. Some testing would be needed.

[GreenComfyTea]

While I am not sure about actual lua modules, reframework does treat all scripts as sharing one global space. So a unique global table that contains functions is totally doable:

CoavinsGlobalFunctions = {
   drawReport = function()
      ...
   end
};

And then calling CoavinsGlobalFunctions.drawReport(). This should work, drawReport function will be accessible thru a global table, while keeping the isolation from other unrelated mods (as long as CoavinsGlobalFunctions is a unique function name).

[Diyagi]

Modules do use the same global namespace, so, any call to reframework functions should work. Now, about the global table (CoavinsGlobalFunctions), i read everywhere to avoid using the global namespace (im not an Lua expert, if you think otherwise, i wanna know !), what i had in mind was:

local coavinsDraw = {}

function coavinsDraw.drawReport()
    ...stuff...
end

return coavinsDraw;

then, in the main script, we can simple require the module:

local coavinsDraw = require "coavins-dps-meter.coavinsDraw"

coavinsDraw.drawReport()

[coavins]

@Diyagi's approach sounds better to me, I don't claim to be a Lua expert either but this sounds more like how Lua modules are meant to be used.

Would different modules be able to call into each other, then? How would it work if we started refactoring things into proper Lua "classes"?

[GreenComfyTea]

I am concerned about return in a global space. Would it break execution of all scripts? Generally speaking in programming it is better to avoid global space but REFramework has its own specifics. By using require you will be calling a module from a script file that has already been executed.

I think that rather that guessing how it would work, it is better to ask REFramework devs directly about modules.

[coavins]

By using require you will be calling a module from a script file that has already been executed.

This is my concern as well. To use modules this way, we would want to know whether REFramework is executing scripts in subdirectories under the autorun directory. Although, a module as described above would not define any globals and simply return itself, so I don't think it would actually have any effect beyond wasting time even if REFramework does execute it.

I think return is fine to use outside of functions, it would just mean REFramework's execution of your script ends early and doesn't establish hooks, callbacks, etc.

[Diyagi]

Reframework will only autorun lua files on root of autorun folder, if its inside an inner folder (like autorun/coavins-dps-meter/script.lua), it will not be executed, unless you require it in a script that its on autorun root folder. Thats aleast what i tested. I actually have a folder called disabled inside autorun where i put scripts that i want to disable.

[GreenComfyTea]

I tried making modules at one point and created a folder inside autorun. But it didn't work. Simply having a folder inside autorun was causing REFramework to throw an error. It was pre-v1.1.1 thou. Good to hear that it works now.

[GreenComfyTea]

Calling require causes the game crash if I press Reload Scripts button. I would advice to wrap require calls with pcall. It fixes the issue. Works during startup initialization thou. Other than that modules work.

[GreenComfyTea]

Would different modules be able to call into each other, then? How would it work if we started refactoring things into proper Lua "classes"?

My understanding of modules is pretty limited but the following information helped me understand it better:

"Lua keeps track of all the files you have required in your code in a table called package.loaded. Every time a file is required, that table is checked, and if the module name exists in the table already, it is not loaded again, but existing module table is used instead. If it does not exist in the table, the module is loaded and the name is added to the table. This way you can require a module many times but it will only be run the first time."

[Diyagi]

Calling require causes the game crash if I press Reload Scripts button. I would advice to wrap require calls with pcall. It fixes the issue. Works during startup initialization thou. Other than that modules work.

Wait, this didnt happened to me, i used Reload many times while i was testing, can you share your module ?

[GreenComfyTea]

Sorry, can't reproduce anymore :/

[coavins]

I was playing around with this idea tonight and I ended up breaking down most of the script into separate modules in this branch. It seems to work surprisingly well with a few occasional bugs.

One thing I don't like about this is that my IDE no longer seems to know whether a function I'm calling on a module actually exists there. As a result of this, I end up calling on the wrong module for a ton of functions (is clearTestData() in core or calc?

It does seem to be a huge improvement on the maintainability and testing fronts, so I think we should continue to pursue this.

[coavins]

@praydog Might you be able to shed some light on this topic? Would you recommend against splitting up an autorun script into modules like this? It looks like it works fine with the current version of REFramework, but I'm concerned about breaking changes down the line if this was not your intention.

reframework/autorun/
  mhrise-coavins-dps.lua
  mhrise-coavins-dps/
    core.lua
    data.lua
    draw.lua
    enum.lua
    report.lua
    state.lua

local STATE  = require 'mhrise-coavins-dps.state'
local CORE   = require 'mhrise-coavins-dps.core'
local ENUM   = require 'mhrise-coavins-dps.enum'
local DATA   = require 'mhrise-coavins-dps.data'
local REPORT = require 'mhrise-coavins-dps.report'
local DRAW   = require 'mhrise-coavins-dps.draw'

Thanks in advance for any insight you can offer.

[GreenComfyTea]

Too late for me dawg. My mod grew too big to be kept in one file. Welcome to the dependency hell, silly me :C

[praydog]

@praydog Might you be able to shed some light on this topic? Would you recommend against splitting up an autorun script into modules like this? It looks like it works fine with the current version of REFramework, but I'm concerned about breaking changes down the line if this was not your intention.

reframework/autorun/
  mhrise-coavins-dps.lua
  mhrise-coavins-dps/
    core.lua
    data.lua
    draw.lua
    enum.lua
    report.lua
    state.lua

local STATE  = require 'mhrise-coavins-dps.state'
local CORE   = require 'mhrise-coavins-dps.core'
local ENUM   = require 'mhrise-coavins-dps.enum'
local DATA   = require 'mhrise-coavins-dps.data'
local REPORT = require 'mhrise-coavins-dps.report'
local DRAW   = require 'mhrise-coavins-dps.draw'

Thanks in advance for any insight you can offer.

It is completely fine if you want to do something like this. We don't have intentions to modify how Lua works. The newer builds also have support for another module style, which implicitly looks for an init.lua in the module folder.

My only recommendation is to ensure that your dependencies are in a separate folder, so they do not get ran automatically, and are only ran explicitly by your script.

You MAY want to look into compiling the lua files (with luac) for final releases, which will not only compile the script, but compile your script and all of your modules into one file. This way you wouldn't need to worry about deleting old files on the user's end if you happen to rename or delete some modules. I'm not sure if REFramework can automatically load .luac files from the autorun folder, but we can add that if this is something you want to try.

[coavins]

My only recommendation is to ensure that your dependencies are in a separate folder, so they do not get ran automatically, and are only ran explicitly by your script.

This was my primary concern, thanks for the tip!

You MAY want to look into compiling the lua files (with luac) for final releases, which will not only compile the script, but compile your script and all of your modules into one file. This way you wouldn't need to worry about deleting old files on the user's end if you happen to rename or delete some modules. I'm not sure if REFramework can automatically load .luac files from the autorun folder, but we can add that if this is something you want to try.

This sounds like a great idea, I didn't know this was an option! I think this would do a lot to simplify the install process and reduce confusion for players. I'll take a look at this and see if it just works already.

edit: I guess not without changes to REFramework. This is definitely something I'd be interested in, though.

[coavins]

Resolved by de09301