Factorio-Access / FactorioAccess

An accessibility mod for the video game Factorio, making the game accessible to the blind and visually impaired.
Other
20 stars 9 forks source link

make a sounds module and pull out all of our magic sound constants to it #216

Open ahicks92 opened 1 month ago

ahicks92 commented 1 month ago

I propose a sound module with functions in it to play sounds, where the function names are semantically named and come with comments as to what changing the function would modify. Right now we randomly just directly play sounds. That's fine until we realize that we may wish to change which sound plays when. We can't if they're random strings in the codebase. We also find sometimes that we want to change other attributes, for example the whole thing about classifying sounds not to muffle in remote view. We should consider a module with functions like this in it:

-- Plays when the cursor is about to align with the bookmark.
function mod.play_ruler_1(pindex, options)
  p.play_sound({ path = "audio-ruler-horizontal-2" } +options) -- pseudocode
end

That is a function which optionally takes options to pass to play_sound and otherwise just knows how to play said sound. + means "merge tables". This seems arduous and annoying. However we can just:

local function decl_play_sound(file)
   return function ()
      -- do the playing here.
   end
end

-- For when the player gets close to the ruler.
mod.ruler_close = decl_play_sound("audio-ruler-horizontal-1")

I have thought about simple constants, however as mentioned above we have cases like the remote view where these wish to come with options For example tuning volumes etc. Using the above pattern it's two lines per sound: a comment (which we would need anyway) and a function materialized from the simple thing above.

This won't make code smaller. Just significantly more changeable and letting people thematically use the right sounds e.g. always using our "the thing wrapped" stuff or whatever. It advertises and describes what sounds are for rather than random file names sprinkled who knows where.

This can be done easily by grepping for play_sound.

Also it can handle pitch and pan if or when we come up with a solution for those, whatever that turns into.

LevFendi commented 1 month ago

I like this idea so that we can add sounds with options. However, for the simple calls with no extra options, I think play_sound can stay as it is because we already get to name the sound paths in data.lua to be as descriptive as we want.

ahicks92 commented 1 month ago

Yeah but the problem is, for example the ruler PR. We use the same sound in multiple places. But they're all strings so you have to change all of them. At the very minimum we need to at least start using shared constants. IMO if we go that far make them functions, but without constants it's hard to change our minds. For example maybe we've got 4 uses for a sound file right now which is 4 constants to the same value, but then later we can just switch one of them to something else by changing the constant to split it to a different file.

I suppose we could just not use functions but for my part I'd love to be able to lay my hands on the full list of, call them sound-nouns given that they'd not necessarily be unique files. Or maybe call it the catalog or something. but along those lines in any case.