otland / forgottenserver

A free and open-source MMORPG server emulator written in C++
https://otland.net
GNU General Public License v2.0
1.57k stars 1.05k forks source link

Fix world light config #4623

Closed ramon-bernardo closed 5 months ago

ramon-bernardo commented 7 months ago

Pull Request Prelude

Changes Proposed

Issues addressed: https://github.com/otland/forgottenserver/issues/4621

omarcopires commented 7 months ago

we had everything centralized in config.lua, now we have several scripts with several different configurations, this is a great return.

Sajgon commented 7 months ago

Why is the config of defaultWorldLight even a thing? there's nothing in the code to handle it being false

Sajgon commented 7 months ago

I think there's another issue with world light @ramon-bernardo

Here it's returned as "worldLightLevel, worldLightColor"

    function Game.getWorldLight()
        return worldLightLevel, worldLightColor
    end

but on other places it's fetched as: local worldLightColor, worldLightLevel = Game.getWorldLight()

amatria commented 7 months ago

we had everything centralized in config.lua, now we have several scripts with several different configurations, this is a great return.

You are absolutely right. It is a pity this parameter is now decentralized. However, #4583 moved the world light algorithm to Lua and so it does not make sense to keep that parameter in the config.lua anymore. But again, I get you. Stating the intent behind this PR would probably reduce the amount of eyebrows raised.

Why is the config of defaultWorldLight even a thing? there's nothing in the code to handle it being false

Not at all. What if I want to make my own world light algorithm? I need this switch to turn the default behavior off. While it is true that there is no code to handle it being false, I could very well add my own.

MillhioreBT commented 5 months ago

If it will no longer be a configuration then make the variable local, since it will only be used in that specific place, if someone changes it to false they know they have to create their own lighting system and they do not have to check if it is false or true elsewhere

Honestly, it doesn't bother me that this boolean is in the Lua configuration, it can be accessed from anywhere and it also doesn't litter the global environment with global variables.