HomeWildfire Games

Add mod selection mod.
Concern RaisedrP15677

Description

Add mod selection mod.

Includes some contributions by rada and sanderd17.

Details

Auditors
elexis
Committed
leperAug 25 2014, 6:02 PM
Parents
rP15676: Add engine support to load mods from config and restart into mods.
Branches
Unknown
Tags
Unknown

Event Timeline

elexis raised a concern with this commit.Nov 30 2017, 12:44 PM
elexis added a subscriber: elexis.

See discussion in rP20552.
Mostly raising a concern for the missing documentation which should be fixed in a separate patch.

/ps/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
1

Misses defining documentation.

7

example inside an example

11

A minority of developers will comprehend this.

28

Contains all mods that are available and not enabled, especially not the ones returned by GetAvailableMods.
Last time I checked disabled is the same as not enabled.

63

pointless

91

Does not return all existing mods

118

So write a ticket

156

Only one copy of the ternary and sort call needed.
The entire switch and compare function could have been collapsed into a single line.

173

Dark gray on dark gray background

175

These properties are mandatory

This commit now has outstanding concerns.Nov 30 2017, 12:45 PM
elexis added inline comments.Dec 1 2017, 5:39 PM
/ps/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
5

Someone should have informed mod developers that they may not used version comparison operators in the name field.
Most of them think that the name property is a label and use all kinds of characters and it's just coincidence that the mod page didn't break yet.

But this field may only be used with dependency checks if different mods may use the same identifier to denote the same set of implemented features. (That hypothetical use case might make more sense with a special field however, so that a mod can state that it implements different features that can be tested against).

elexis accepted this commit.Dec 15 2017, 5:05 PM

Besides the confusing GUI page name the defects should be fixed following rP20552, rP20637 and some others. Thanks for giving the user an easy way to enable and disable mods!

/ps/trunk/source/ps/scripting/JSInterface_Mod.cpp
46
All concerns with this commit have now been addressed.Dec 15 2017, 5:05 PM

Thanks for giving the user an easy way to enable and disable mods!

Thanks for cleaning this up, breaking parts of it, not caring about invalidating possibly not obvious parts of the design instead of figuring out or even just asking to clarify that, then being smug about having done so. Have fun mainta^W breaking this more.

/ps/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
118

So we should open a ticket for every TODO in the whole codebase? Really?

elexis raised a concern with this commit.Dec 28 2017, 10:14 PM
elexis added inline comments.
/ps/trunk/binaries/data/mods/mod/gui/modmod/modmod.js
88

This doesn't work as expected.

Because neither this nor enableMod update the user config, the mod page after the restart only displays the list of mods enabled in the user.cfg, not the list of mods that are currently actually enabled. It should display the same list before and after the restart, just that the mods have been actually enabled after the restart.

Reproduce:

  1. Open the mod page.
  2. Disable the public mod.
  3. Notice the list of enabled mods is empty.
  4. Click on "Start mods"
  5. Press F9 and type warn(uneval(Engine.GetEngineInfo()));`
  6. Notice the public mod is not enabled but listed as enabled.

Described in #4881.

<Insert guilt flamewar here>

(The restarting is done without restarting the process but just restarting the main loop in main.cpp which calls CONFIG_Init in source/ps/GameSetup/Config.cpp which reloads the user.cfg, so might not be a cakewalk to fix)

This commit now has outstanding concerns.Dec 28 2017, 10:14 PM
elexis resigned from this commit.Dec 28 2017, 10:14 PM
This commit no longer requires audit.Dec 28 2017, 10:14 PM
elexis raised a concern with this commit.Oct 23 2018, 5:37 PM

Enabling a mod when a filter is active does not remove the mod from the list of unenabled mods, thus misleads the user into believing the mod is not activated. Then after some multiple clicks on enabling it will trigger a debug breakpoint in the engine complaining about OOB access.

This commit now has outstanding concerns.Oct 23 2018, 5:37 PM