Request: Change ShowError() to ShowWarning() on map startup loading of invalid modules

[CatsEyeXI]

Hello,

I'd like to propose a small change to moduleutils.cpp
https://github.com/LandSandBoat/server/blob/base/src/map/utils/moduleutils.cpp#L254

This check was implemented two weeks ago:
310b69d

We have several modules for defining enums, where they don't actually override anything, and I think this is a valid use case for modules.
Sadly, throwing the 'error' text into the map server log causes the CI to fail.
Would you be open to changing the call from ShowError() to ShowWarning(), or making these prints configurable by setting?

Thanks

[zach2good]

A module is defined by having both a name and a collection of overrides. If it doesn't have both of these it is considered invalid. Loading an invalid module is an error.
https://github.com/LandSandBoat/server/blob/base/modules/module_utils.lua#L59

If you're using modules/init.txt to specify a folder containing many folders and files, including non-modules, make your init.txt more specific or move the non-modules to somewhere else.

Less specific

init.txt

custom/

# Will load everything in:
# <repo>
#     modules/
#         custom/

More specific

init.txt

custom/my_module.lua

# Will load only these files
# <repo>
#     modules/
#         custom/
#.            my_module.lua

Moving non-modules out of modules/

your_enum.lua

-- custom namespace
cn = cn or {}

cn.testEnum =
{
    A_VALUE  = 0,
    THE_VALUE = 1,
}

-- Not returning anything

test_module.lua

require('modules/module_utils')
require('modules/enums/your_enum')

local m = Module:new('test_module') -- name

m:addOverride('xi.player.charCreate', function(player) -- override
    super(player)

    -- Some module logic
end)

return m -- return

init.txt

custom/

# `modules/custom` will be loaded, `modules/enums` won't - but it'll be `require()`'d from inside your module.
# <repo>
#     modules/
#         enums/
#             your_enum.lua
#         custom/
#             test_module.lua
#             <all your other modules>

[zach2good]

Something similar actually came up internally recently with the renamer module, where we have a list.lua file containing a non-module file of name data: https://github.com/LandSandBoat/server/blob/base/modules/renamer/lua/list.lua

The answer here was to load renamer/cpp in init.txt instead of renamer/.

[zach2good]

Having explained all of this, I'm now happy with this layout and this comment I've left behind becomes moot and can be removed:
310b69d#diff-e8c901ba1d891b4a7ecf24317205c9f0c49d7e76066d599086019a750bfa36e6R262

We shouldn't be turning off or downgrading legitimate errors in the loading process.

EDIT: In the case of the comment I linked, we don't need to differentiate between empty or data-only "modules" in the load process, because they shouldn't be a part of the load process and it's very easy to not load them.

[CatsEyeXI]

Thank you