Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lava: includable in mapconfig, new config options, some refactor #3964

Open
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

saurtron
Copy link
Collaborator

@saurtron saurtron commented Nov 23, 2024

Work done

  • Refactor lavaConfig.lua module into Spring.Lava so it can be easier to access (only available to LuaUI and LuaRules for now).
  • Make lava config includable in mapconfig/lava.lua inside mapfiles.
  • Add more lava config options:
    • The options were already inside map_lava.lua as TODO notes and comments.
    • See at the bottom of this PR for more details.
    • damageFeatures: enable damage to features and control damage speed to features.
    • effectDamage: set custom ceg effect for damage.
    • effectBurst: set custom ceg effect for bursts (or disable).
    • effectBurstSounds: set custom sounds for bursts.
    • ambientSounds: set custom ambient sounds (or disable).
  • Slight cleanup of the lava code.
    • Extracted shaders to shaders/GLSL/lava/.
    • No more lava globals from the lavaConfig.lua (now lava.lua) module.
    • Optimized most critical parts from map_lava.lua a bit (tried not to overdo it).
  • (maybe) Fixes stronghold lava effect. - actually this needs clarification

Test steps

  • Try your favourite lava map with this PR and check it still works the same (I tried mostly ghenna since that has a lava cycle).

You can try some of the new config options by editing modules/lava.lua directly, look for the if relevant to your map.

Examples:

elseif string.find(mapName, "ghenna") then
  damageFeatures = true -- features will be damaged by lava
  effectDamage = "commander-spawn" -- commander spawn effect when a unit is damaged
  effectBurst = "commander-spawn" -- commander-spawn effect randomly around lava
  effectBurstSounds = {{"commanderspawn", 20, 60}, {"beep6", 40, 70}}  -- random sounds for bursts, not these ones don't work well since they're not mono and thus can't be positioned in 3d
  ambientSounds = {{"commanderspawn", 20, 60}, {"beep6", 40, 70}} -- some random ambient sounds

Discussion

Stronghold fix

  • According to @Nikuksis it's supposed to look like I'm setting it since it was changed to acid with the map remake this year.
    • There were two config stances, and the one not being applied seems to be the one added later and also the good one according to the lobby pic. Actually, there seems to be some fighting over it's style:

Namespace

  • Spring.Lava might be temporary, I'd like to introduce Spring.Map to access other common functionality like getWaterLevel
    • I actually started this because I wanted to homogenize determining map water level/status in different places since it's not being done 100% correct everywhere and there's quite a few of duplicated code as well. Spring.Lava.isLavaMap is one important part of determining water status.
    • Can later do Spring.Lava and Spring.Map, or put lava inside map as Spring.Map.lava or Spring.Map.Lava. Also would like to be able to Spring.Map.isLavaMap directly.
    • I moved the luarules/configs/lavaConfig.lua code to modules/lava.lua. I think it's better there, but this is breaking change since api users might be VFS.including the old path. It can be moved back if needed. Also if more map stuff gets added like I say above it can be better to move it into modules/map/lava.lua.
    • I could introduce an empty Map module in this PR to leave a more final api and paths.

Including in maps

  • With lava now includable in maps, the map specific config can now be migrated from the game to the mapfiles and removed from the codebase.
  • I can migrate the config to the mapfiles, but have no idea atm where the code for them lives atm :P
  • It will be simple, since creators just need to copy the config from common/configs/LavaMaps/ to mapconfig/lava.lua.

Config options

  • There are more config options that could be added, but for now I tried to do just the ones I saw at the map_lava.lua file with comments like "TODO make this configurable". I have no special feeling about the options, they can be removed or changed.
    • Could make more bursts per second, now the ratio is fixed.
    • Could have arrays of cegdamage and cegburst instead of just one to choose from (it can be later upgraded to allow just one element or an array if the need arises).

New shaders location

  • I extracted shaders from the file since they make working on the lua stuff very annoying and also looks cleaner this way. Would like to hear from @Beherith to see if he likes where I placed them or even getting them out of the file. Don't want to step into other ppl work.

Others

  • There was code in the file to destroy features, but it was using SetFeatureReclaim, that doesn't update the gui very well so not sure if that's the best way or there's something else I'm missing. I changed it to SetFeatureResources where I can actually set metal, energy and reclaimLeft so it all updates properly at gui tooltip.
  • Not sure the cegdamage effects play very well with features, there might be something going on there, but likely it's a limitation of ceg or just me being paranoid.
  • I noticed an exception for air units so flying units won't die in lava even if not flying. This may be some kind of optimization. It's a bit weird landed units won't die, but didn't touch this for now since it doesn't look so important.
  • While I refactored and reorganized a bit, I tried not to overdo it and there's a few things specially in map_lava.lua that can still be improved or optimized. Generally I have modified the tightest loops or code around what I already had to modify anyways.

Screenshots:

Before and after comparison:

(should look the same, both images taken at 1:50 in game time)

lava-looks-same

Destroyable features

lava-damages-features

Custom ceg damage effect

testing random cegs from around the codebase

lava-custom-ceg-damage

Custom ceg burst effect

testing random cegs from around the codebase

lava-custom-ceg-bursts

Stronghold fix

This is how it looks in each version.

lava-stronghold

Detailed new options

damageFeatures

Enable damage to features and control damage speed. This works by setting it to true or a float. true will actually set 0.1. The value is the proportion of resources from the feature destroyed per second. So 0.1 means it will get destroyed in 10 seconds.

Default: false

effectDamage

Set custom ceg effect for damage. This is the effect applied when a unit is damaged or destroyed.

Default: "lavadamage"

effectBurst

Set custom ceg effect for bursts (or disable). A burst appears somewhere in the map every now and then.

Can be set to false and then no bursts will appear.

Default: "lavasplash"

effectBurstSounds

Set custom sounds for bursts. An array of sounds with min and max volume, one of them will be randomly selected every time a burst happens.

Default: { {"lavaburst1", 80, 100}, {"lavaburst2", 80, 100} }

ambientSounds

Set of custom ambient sounds. An array of sounds with min and max volume. One of them will appear somewhere in the map every now and then. Can be set to false to disable ambient sounds.

Default:

{ {"lavabubbleshort1", 25, 65},
   {"lavabubbleshort2", 25, 65},
   {"lavarumbleshort1", 20, 40},
   {"lavarumbleshort2", 20, 40},
   {"lavarumbleshort3", 20, 40} }

Also move voidWaterMap parsing to lava.lua.
…fectAmbientSounds.

- effectDamage: controls the damage ceg effect
- effectBurst: controls the ambient bursts ceg effect
- effectBurstSounds: to define sounds for bursts
- effectAmbientSounds: to define lava ambient sounds
…minhealth can be combined with either direct or proportional modes.
modules/lava.lua Outdated Show resolved Hide resolved
@p2004a p2004a requested a review from Beherith November 23, 2024 18:37
@p2004a
Copy link
Collaborator

p2004a commented Nov 23, 2024

How often is map lava config modified in comparison to the rest of map file?

I assume this feature also means that texture will need to be distributed inside every map file so that it's all self contained?

The one thing I care about is that map files will become only more and more pure structured data, no code, no logic.

@saurtron
Copy link
Collaborator Author

How often is map lava config modified in comparison to the rest of map file?

No idea, but giving the map makers options to include the lava config will likely help them in tweaking it more.

I assume this feature also means that texture will need to be distributed inside every map file so that it's all self contained?

It can be distributed inside the map file if it's not available inside bar already, yes.

The one thing I care about is that map files will become only more and more pure structured data, no code, no logic.

This is just some structured data so I guess it's ok but let me know if you have any specific concerns.

make game map configuration loaded from common/configs/LavaMaps with
files for every map.
@saurtron saurtron changed the title Lava: includable in mapinfo, new config options, some refactor Lava: includable in mapconfig, new config options, some refactor Nov 24, 2024
@saurtron
Copy link
Collaborator Author

saurtron commented Nov 24, 2024

Moved all map specific configuration to common/configs/LavaMaps/. I'm introducing common/configs, not sure if that's the best place, but I don't like luarules/configs/ since being available to LuaUI, and loaded by a common module, I don't think this is specific to luarules. I can move it there if you prefer.

While doing this I noticed the following:

  • Stronghold had two config definitions, looks like the latter one is the good one (it's acid instead of lava, like the map pic), but the game was loading the first one due to the ifdef structure. I have added the latter instead so this may be fixing the Stronghold map.
  • Pit of Azar is in the configs, but the map is not provided atm by BAR. Somebody added the config at some point, not sure what to make of that. I have added the config but it will likely need to be tweaked if it gets added. I checked and it works but looks a bit too tiled after the map edges.
  • Both "Seths Ravine Remake" and "MoonQ20XR2" maps had configs with mapLava = false set, this would make the lavaConfig not parse it. I'm not sure if they're supposed to have lava or damageWater in some way, but I checked and doesn't seem to be the case.

@saurtron
Copy link
Collaborator Author

The one thing I care about is that map files will become only more and more pure structured data, no code, no logic.

Actually, thinking about this, then maybe the config should be provided in the map as json files or similar, instead of lua files?

Thing is I don't see mapfiles using json files atm, so that should likely be done as a separate project to remove all lua from maps. Also maybe the project wants to use a different format instead of json if it ever goes in that direction.

@p2004a
Copy link
Collaborator

p2004a commented Nov 24, 2024

No idea, but giving the map makers options to include the lava config will likely help them in tweaking it more.

Map files don't have incremental updates, you have do download whole new file, so if lava config is updated often, we might not want to have it in map files. Looking at the file history it doesn't seem to change very often, at least not the per-map values.

Also

I can migrate the config to the mapfiles, but have no idea atm where the code for them lives atm :P

It kinda doesn't. There aren't really repos for maps AFAIK, definitely not for majority of them. I don't know how mappers are managing their maps, but it's not like we can send a PR somewhere to change particular map. Mappers are directly uploading the map archives themselves, so I think bulk change is you download map archive, modify values, pack it, upload, notify mapper to update their local files... or spam mappers... But it's also ok to just require it for new maps, but for old ones still keep them in the game.

Pit of Azar is in the configs, but the map is not provided atm by BAR. Somebody added the config at some point, not sure what to make of that

The map was released but then reverted because there were issues with it.

Actually, thinking about this, then maybe the config should be provided in the map as json files or similar, instead of lua files? (...)

Yes, but as you say that should be separate project, for now I care there is no logic, so the question is effectively "Is it possible to put this into JSON", if the answer is "no", then IMHO it should not live there.


Anyway, I think moving away from per-map configuration inside of the game is good. It is problematic for all sorts of reasons:

  • potential custom maps
  • once we switch to slower release process where there is stable and test version, game releases will block releases of maps

The alternative is to put this information in the out-of-mapfile maps metadata so just like start pos suggestions are done (passed in via modoption), how we want ffa startpos to be (currently also in game repo), how startboxes are configured etc. IMHO it needs Beherith input. The more dynamic this data is, the better it fits into maps metadata, the more static it is per map version, the more it belongs in the map files.

@Beherith
Copy link
Collaborator

Fuck I totally forgot about this one: https://github.com/beyond-all-reason/Beyond-All-Reason/commits/Lava-Rework/

@Beherith
Copy link
Collaborator

Regarding map/game metadata

My smartest approach so far was done for map_grass, where the config load order, from mapinfo.lua is:

  1. Load config from map
  2. Override from game if game override is present

The new cegs, while welcome, look strange as white/purple, @icexuick ?

@saurtron
Copy link
Collaborator Author

saurtron commented Nov 24, 2024

My smartest approach so far was done for map_grass, where the config load order, from mapinfo.lua is:

  1. Load config from map
  2. Override from game if game override is present

Exactly what I'm doing. I think it's best too.

The new cegs, while welcome, look strange as white/purple

They're not new XD, I just tried with random cegs I found around the codebase (scavenger spawn and commander spawn I think). What I did is just allow maps to specify some available ceg to use instead of the default one through config.

@saurtron
Copy link
Collaborator Author

Fuck I totally forgot about this one: https://github.com/beyond-all-reason/Beyond-All-Reason/commits/Lava-Rework/

lol, I can take a look and merge any changes into a branch if this gets merged, if you want, shouldn't be a problem

@Beherith
Copy link
Collaborator

Fuck I totally forgot about this one: https://github.com/beyond-all-reason/Beyond-All-Reason/commits/Lava-Rework/

lol, I can take a look and merge any changes into a branch if this gets merged, if you want, shouldn't be a problem

Nah, I merged the main part of it in the commits above, namely the reload and fps smoothing

@WatchTheFort
Copy link
Member

I think it would be better to consolidate all of a map's config code into a single file, with one file per map. We can create a dedicated directory e.g. maps/.

This would be better than splitting a single map's config across multiple files.

@saurtron
Copy link
Collaborator Author

saurtron commented Dec 12, 2024

I think it would be better to consolidate all of a map's config code into a single file, with one file per map. We can create a dedicated directory e.g. maps/.

This would be better than splitting a single map's config across multiple files.

Are there other configs around? Or should I just place the ones here under maps/ ?

edit: Ok I see there are others but imo joining those should be done as a separate PR since that will require mangling a lot of files and possibly some code too.

luaui/Widgets/map_edge_extension2.lua Show resolved Hide resolved
luarules/gadgets/map_lava.lua Outdated Show resolved Hide resolved
common/configs/LavaMaps/Ghenna Rising.lua Show resolved Hide resolved
modules/lava.lua Outdated Show resolved Hide resolved
@Beherith
Copy link
Collaborator

For the record, anyone adding a file into bar thats called config.lua will be taken behind the chemical shed and executed.

@Damgam
Copy link
Member

Damgam commented Dec 15, 2024

I'm just gonna say this.

I don't like some of these config options being available. The only configurable stuff should be the numbers, not the mechanics. Stuff like "damageMode" shouldn't be an option.

damageMinHealth is outright against the design of lava - it should not allow units to stay in it alive for long.

features should always be killed by lava. Why make that an option?

@saurtron
Copy link
Collaborator Author

saurtron commented Dec 15, 2024

features should always be killed by lava. Why make that an option?

I just implemented those options that seemed already there in the code but commented with (TODO make this configurable).

That one tbh I think was just a bad (commented) implementation (that's why I also added the proportional damage, that seemed more in line with what the comment author maybe had in mind), anyways I can remove any of the options you don't like, I have no specific feelings about those.

Also, those (damage and damageMinHealth) options are for units, not for features, for now I just left proportional damage system for features since that's also what was already commented, I just made the proportion amount per second configurable (through damageFeatures that also controls disabling damage to features completely -its actually disabled by default-).

edit: that one is from

--This should be in config file to change damage + effects/cegs
-- local health, maxhealth = Spring.GetUnitHealth(all_units[i])
-- Spring.AddUnitDamage (all_units[i], health - maxhealth*0.033, 0, Spring.GetGaiaTeamID(), 1)
Spring.AddUnitDamage (all_units[i], lavaDamage/3, 0, Spring.GetGaiaTeamID(), 1)
--Spring.DestroyUnit (all_units[i], true, false, Spring.GetGaiaTeamID())
Spring.SpawnCEG("lavadamage", x, y+5, z)

@sprunk
Copy link
Collaborator

sprunk commented Dec 15, 2024

Configurable probably means customparams. For example lava immunity can be useful for built-in map rocks that get flooded by dynamic lava but survive and are still there when it cycles back down.

@saurtron
Copy link
Collaborator Author

I don't like some of these config options being available. The only configurable stuff should be the numbers, not the mechanics. Stuff like "damageMode" shouldn't be an option.

Done, removed the damageMode and damageMinHealth.

local lavaDamageMode = lava.damageMode

-- damage is specified in health lost per second, damage is applied every 10 frames
local lavaDamage = lava.damage/3.0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might want to do

local DAMAGE_RATE = 10 -- frames
local lavaDamage = lava.damage * (DAMAGE_RATE / Game.gameSpeed)

and then if frame % DAMAGE_RATE == 0 somewhere below where the magic constant 10 currently lives

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants