> You're missing likely missing one or more techniques that make this work well:...

throwaway2037 · on May 27, 2021

I have seen so much GUI code like this in my career! Real world sophisticated GUIs can have tens or hundreds of attributes to setup. Especially ancient Xlib stuff, this was the norm. You have a few functions with maybe hundreds of lines doing pure GUI setup. No problem -- easy to mentally compartmentalise.

Your deeper point (if I may theorise): Stop following hard-and-fast rules. Instead, do what makes sense and is easy to read and maintain.

joshka · on May 27, 2021

> I know how to do it, I just don't always think it's worth it.

Agreed:)

Generally no problem with this method other than it's difficult to at a glance see what each item will get set to. Something like the following might be an easy first step:

    function getFullParameters() {
      function param(defaultValue, redraws) {
        return { defaultValue: defaultValue, randomFn: null, redraws };
      }
      function param(defaultValue, randomFn, redraws) {
        return { defaultValue: defaultValue, randomFn: randomFn, redraws };
      }
      const result = {
        "gridType": param(1, onlyOneRedraw("grid")),
        "gridSize": param(32, onlyOneRedraw("grid")),
        "gridOpacity": param(40, onlyOneRedraw("grid")),
        "width": param(1024, allRedraws()),
        "height": param(1024, allRedraws()),
        "seed": param(1, () => Math.round(Math.random() * 65536), allRedraws()),
        "treeDensity": param(40, () => Math.round(Math.random() * 100), onlyOneRedraw("trees")),
        "stoneDensity": param(40, () => Math.round(Math.random() * 20 * Math.random() * 5), onlyOneRedraw("stones")),
        "twigsDensity": param(40, () => Math.round(Math.random() * 20 * Math.random() * 5), onlyOneRedraw("twigs")),
        "riverSize": param(3, () => Math.random() > 0.5 ? Math.round(Math.random() * 10) : 0, onlyRedrawsAfter("river")),
        "roadSize": param(0, () => Math.random() > 0.5 ? Math.round(Math.random() * 10) : 0, onlyRedrawsAfter("river")),
        "centerRandomness": param(20, () => Math.round(30), onlyOneRedraw("trees")),
        "leavedTreeProportion": param(95, () => Math.round(Math.random() * 100), onlyOneRedraw("trees")),
        "treeSize": param(50, () => Math.round(30) + Math.round(Math.random() * 40), onlyOneRedraw("trees")),
        "treeColor": param(120, () => Math.round(Math.random() * 65536), onlyOneRedraw("trees")),
        "treeSeparation": param(40, () => Math.round(80 + Math.random() * 20), onlyOneRedraw("trees")),
        "serrationAmplitude": param(130, () => Math.round(80 + Math.random() * 40), onlyOneRedraw("trees")),
        "serrationFrequency": param(30, () => Math.round(80 + Math.random() * 40), onlyOneRedraw("trees")),
        "serrationRandomness": param(250, () => Math.round(100), onlyOneRedraw("trees")),
        "colorRandomness": param(30, () => Math.round(20), onlyOneRedraw("trees")),
        "clearings": param(9, () => Math.round(3 + Math.random() * 10), onlyRedrawsAfter("clearings")),
        "clearingSize": param(30, () => Math.round(30 + Math.random() * 20), onlyRedrawsAfter("clearings")),
        "treeSteps": param(2, () => Math.round(3 + Math.random() * 2), onlyOneRedraw("trees")),
        "backgroundNo": param(1, onlyTheseRedraws(["background", "backgroundCover"])),
        "showColliders": param(0, onlyOneRedraw("colliders")),
        "grassLength": param(85, () => Math.round(25 + Math.random() * 50), onlyTheseRedraws(["background", "backgroundCover"])),
        "grassDensity": param(120, () => Math.round(25 + Math.random() * 50), onlyTheseRedraws(["background", "backgroundCover"])),
        "grassSpread": param(45, () => Math.round(5 + Math.random() * 25), onlyTheseRedraws(["background", "backgroundCover"])),
        "autoredraw": { defaultValue: true, randomFn: null, redraw: noneRedraws(), },
      };
      return result;
    }

For someone looking at this for the first time, the rationale for each random function choice is obtuse so you might consider pulling out each type of random function into something descriptive like randomIntUpto(65536), randomDensity(20, 5), randomIntRange(30, 70).

Does it add value? Maybe - ask a junior to review the two and see which they prefer maintaining. Regardless, this code mostly exists at a single level of abstraction, which tends to imply simple refactorings rather than complex.

My guess is if this extended to multiple (levels / maps / ?) you'd probably split the settings into multiple functions, one per map right...?

ajuc · on May 28, 2021

> My guess is if this extended to multiple (levels / maps / ?) you'd probably split the settings into multiple functions, one per map right...?

This was handling ui dependencies for https://ajuc.github.io/outdoorsBattlemapGenerator/

Basically I wanted to redraw as little as possible so I build a dependency graph.

But then I wanted to add more parameters and to group them, so I can have many different kinds of trees without hardcoding their parameters. It was mostly an UI problem, not a refactor problem. So I'm rewriting it like this:

https://ajuc.github.io/kartograf/

Graph editor keeps my dependencies for me, and user can copy-paste 20 different kinds of trees and play with their parameters independently. And I don't need to write any code - a library handles it for me :)

Also now i can add interpolate node which takes 2 configurations and a number and interpolates the result between them. So I can have high grass go smoothly to low grass while trees go from one kind to another.