`gui/mod-manager` improvements #1481

ong-yinggao98 · 2025-07-12T20:38:26Z

Requires DFHack/dfhack#5500 be merged for arena mode views

Fixes Mod Manager does not automatically update vanilla mods dfhack#4799
Fixes gui/mod-manager should be able to automatically apply mods to the Object Testing Arena dfhack#4402

The manager also notifies users when a version of a vanilla module in their preset has been changed. This way they should know to update the preset to avoid the popup in future. Since mod manager no longer gives a hoot about the vanilla module versions, an entirely vanilla preset for 51.13 should theoretically also be backwards compatible with an older version.

TODO

~~Mod Manager is duplicating mods to both inactive and active lists. dfhack#3746~~ Too much for this PR, I'll likely tackle it next time.

ab9rf · 2025-07-12T21:09:55Z

I suspect the reason why fields.src_dir[i].value:startswith('data/vanilla') does not work is because fields.src_dir[i].value is a C++ string (being accessed via proxy), not a Lua string, and so the Lua method startswith (which is defined for Lua strings but not for C++ strings) doesn't work.

ong-yinggao98 · 2025-07-12T21:21:45Z

I suspect the reason why fields.src_dir[i].value:startswith('data/vanilla') does not work is because fields.src_dir[i].value is a C++ string (being accessed via proxy), not a Lua string, and so the Lua method startswith (which is defined for Lua strings but not for C++ strings) doesn't work.

Interesting, but in that case why does it work in script-manager.lua::get_active_mods()? It seems to call startswith just fine and the target seems to also be a C++ string.

-- local ol = df.global.world.object_loader
        local path = ol.object_load_order_src_dir[idx].value
        table.insert(mods, {
            id=ol.object_load_order_id[idx].value,
            name=ol.object_load_order_name[idx].value,
            version=ol.object_load_order_displayed_version[idx].value,
            numeric_version=ol.object_load_order_numeric_version[idx],
            path=path,
            vanilla=path:startswith('data/vanilla/'), -- windows also uses forward slashes
        })

SilasD · 2025-07-12T21:34:45Z

maybe forcing it into a Lua string would work? untested code.

tostring(fields.src_dir[i].value):startswith('data/vanilla')

Edit: Your comment in gui/mod-manager.lua shows this code:
from_fields.src_dir[i].startswith('data/vanilla')
That doesn't work the same as this edited code:
from_fields.src_dir[i]:startswith('data/vanilla')
The . was changed to :, to comply with Lua's object-oriented style string functions.

ab9rf · 2025-07-13T00:28:09Z

Interesting, but in that case why does it work in script-manager.lua::get_active_mods()? It seems to call startswith just fine and the target seems to also be a C++ string.

-- local ol = df.global.world.object_loader
        local path = ol.object_load_order_src_dir[idx].value
        table.insert(mods, {
            id=ol.object_load_order_id[idx].value,
            name=ol.object_load_order_name[idx].value,
            version=ol.object_load_order_displayed_version[idx].value,
            numeric_version=ol.object_load_order_numeric_version[idx],
            path=path,
            vanilla=path:startswith('data/vanilla/'), -- windows also uses forward slashes
        })

Because local path = ol.object_load_order_src_dir[idx].value results in it being converted from a C++ string to a Lua string, and so path:startswith is looking for that named method on a Lua string.

This is an artifact of how our C++ object proxying into Lua works. When you directly "read" from a C++ string in order to use its value as a Lua value, the proxy handler automatically converts it into a Lua string. However, when you call a method directly on an object, this doesn't happen: instead, the metatable of the C++ proxy object is consulted, and as there is no startswith method in the C++ string proxy object metatable, the method call fails.

It might be an idea to "chain" the Lua string metatable onto the C++ string proxy metatable, but I'm not at all sure how to go about doing this. Will require considerable thought.

Copying the proxy object into a local variable before calling the method should provide a workaround since the C++ proxy object will be converted to a Lua object at that time.

ong-yinggao98 · 2025-07-13T08:35:34Z

I haven't tried SilasD's suggestion, but ab9rf's works like a charm.

gui/mod-manager.lua

ab9rf · 2025-07-25T17:06:19Z

I just tested this locally with 52.02 and the arena mode functionality does not work. It's possible that the viewscreen name has changed in 52.xx, but that needs to be addressed before I can consider merging this.

SilasD · 2025-07-25T18:10:34Z

As a minor note, Lua supports multiple return values.
Lua 5.3 Reference Manual, section 3.3.4

Functions can return more than one value, (... specific syntax ...)

I'm going to go through and flag these, with suggested code.

gui/mod-manager.lua

lethosor · 2025-07-25T18:25:47Z

@ab9rf

I suspect the reason why fields.src_dir[i].value:startswith('data/vanilla') does not work is because fields.src_dir[i].value is a C++ string (being accessed via proxy), not a Lua string, and so the Lua method startswith (which is defined for Lua strings but not for C++ strings) doesn't work.

This doesn't really make sense to me. C++ strings returned through the Lua wrapper are Lua strings.

string* is different - that is exposed as a pointer with a .value field, which is a string. (Maybe this is what you meant by "proxy object"?)

I see this in the PR:

local function vanilla(dir)
    dir = dir.value
    return dir:startswith('data/vanilla')
end

which appears to only work on string*s, because it uses .value but I see no reason why startswith() wouldn't work once the string has been dereferenced.

SilasD · 2025-07-25T18:26:11Z

gui/mod-manager.lua

            mod_index = i
            break
        end
    end

    if mod_index == nil then
-        return false
+        return { success= false, version= nil }


Consider
return false, nil

I think either is fine. And I think there is a decent argument for using "named" return values like this instead of unnamed ones (multiple returns)

ab9rf · 2025-07-25T18:41:12Z

This doesn't really make sense to me. C++ strings returned through the Lua wrapper are Lua strings.

Only if they pass through stl_string_identity::lua_write. My best guess, based on the observed behavior, is that this isn't happening when the object is being pulled out of a "container" object; instead of ultimately pushing a Lua string onto the stack, it's pushing the Lua userdata object that proxies the C++ string without using the lua_write specialization that converts C++ strings into Lua strings. It has to retain the proxy object so that if it gets assigned to the assignment uses the STL string assignment method. Otherwise you wouldn't be able to do object.field = "string" and get a useful result.

gui/mod-manager.lua

lethosor · 2025-07-25T20:15:11Z

Only if they pass through stl_string_identity::lua_write. My best guess, based on the observed behavior, is that this isn't happening when the object is being pulled out of a "container" object; instead of ultimately pushing a Lua string onto the stack, it's pushing the Lua userdata object that proxies the C++ string without using the lua_write specialization that converts C++ strings into Lua strings. It has to retain the proxy object so that if it gets assigned to the assignment uses the STL string assignment method. Otherwise you wouldn't be able to do object.field = "string" and get a useful result.

Ok, so what I'm trying to say is that there are two cases:

Native Lua strings, equivalent to "foo" (also returned by reading std::string fields from C++), where startswith() works
Proxy objects, with a .value field, defined by DFHack and returned when reading std::string* - startswith() does not work on these natively, but does work on the .value field

And there isn't a third case that looks like case 1 but where startswith() doesn't work. (i.e. this is unlike Java/JavaScript where there is a distinction between string literals and new String())

ong-yinggao98 · 2025-07-25T20:25:27Z

I just tested this locally with 52.02 and the arena mode functionality does not work. It's possible that the viewscreen name has changed in 52.xx, but that needs to be addressed before I can consider merging this.

Correct, please do not merge it yet. I had mentioned that I need the the related PR merged but I'll move it to the top of the description in case just for more prominence.

Requires DFHack/dfhack#5500

Co-authored-by: SilasD <[email protected]>

ab9rf · 2025-07-25T20:37:04Z

I just tested this locally with 52.02 and the arena mode functionality does not work. It's possible that the viewscreen name has changed in 52.xx, but that needs to be addressed before I can consider merging this.

Correct, please do not merge it yet. I mentioned that I need the the related PR merged but I'll move it to the top of the description in bold.

Requires DFHack/dfhack#5500

Gotcha, I will go look at that PR now.

ong-yinggao98 · 2025-07-25T20:40:47Z

I'm going to go through and flag these, with suggested code.

In hindsight unpacked return types should have been obvious to me, thanks a ton for the comments and suggestions.

ab9rf · 2025-07-25T20:51:13Z

I just tested this locally, after merging DFHack/dfhack#5500, and while the panel appears in arena mode and I can save profiles I cannot load a profile.

I suspect that this needs updates to reflect the type changes with 52.02; if you look at the changes made for 52.02 compatibility you will note that I had to make adjustments to code in script-manager.lua and I suspect parallel changes need to be made here.

E:/Kelly/DF/df_52_02_steam/hack/scripts/gui/mod-manager.lua:17: attempt to index a nil value (local 'dir')
stack traceback:
        E:/Kelly/DF/df_52_02_steam/hack/scripts/gui/mod-manager.lua:17: in upvalue 'vanilla'
        E:/Kelly/DF/df_52_02_steam/hack/scripts/gui/mod-manager.lua:89: in function <E:/Kelly/DF/df_52_02_steam/hack/scripts/gui/mod-manager.lua:78>
        (...tail calls...)
        E:/Kelly/DF/df_52_02_steam/hack/scripts/gui/mod-manager.lua:144: in upvalue 'swap_modlist'
        E:/Kelly/DF/df_52_02_steam/hack/scripts/gui/mod-manager.lua:248: in upvalue 'load_preset'
        E:/Kelly/DF/df_52_02_steam/hack/scripts/gui/mod-manager.lua:345: in local 'cb'
        E:\Kelly\DF\df_52_02_steam\hack\lua\gui\widgets\list.lua:323: in function 'gui.widgets.list.double_click'
        E:\Kelly\DF\df_52_02_steam\hack\lua\gui\widgets\list.lua:345: in function 'gui.widgets.list.onInput'
        E:\Kelly\DF\df_52_02_steam\hack\lua\gui.lua:869: in function <E:\Kelly\DF\df_52_02_steam\hack\lua\gui.lua:856>
        (...tail calls...)
        ...df_52_02_steam\hack\lua\gui\widgets\containers\panel.lua:306: in function 'gui.widgets.containers.panel.onInput'
        E:\Kelly\DF\df_52_02_steam\hack\lua\gui.lua:869: in function <E:\Kelly\DF\df_52_02_steam\hack\lua\gui.lua:856>
        (...tail calls...)
        E:\Kelly\DF\df_52_02_steam\hack\lua\gui.lua:1119: in function <E:\Kelly\DF\df_52_02_steam\hack\lua\gui.lua:1103>
        [C]: in ?

ab9rf · 2025-07-25T20:53:37Z

Only if they pass through stl_string_identity::lua_write. My best guess, based on the observed behavior, is that this isn't happening when the object is being pulled out of a "container" object; instead of ultimately pushing a Lua string onto the stack, it's pushing the Lua userdata object that proxies the C++ string without using the lua_write specialization that converts C++ strings into Lua strings. It has to retain the proxy object so that if it gets assigned to the assignment uses the STL string assignment method. Otherwise you wouldn't be able to do object.field = "string" and get a useful result.

Ok, so what I'm trying to say is that there are two cases:

Native Lua strings, equivalent to "foo" (also returned by reading std::string fields from C++), where startswith() works

Proxy objects, with a .value field, defined by DFHack and returned when reading std::string* - startswith() does not work on these natively, but does work on the .value field

And there isn't a third case that looks like case 1 but where startswith() doesn't work. (i.e. this is unlike Java/JavaScript where there is a distinction between string literals and new String())

I understand what you're saying now. I'm still not fully up to speed with all the metamethods on some of the proxy objects.

ong-yinggao98 · 2025-07-25T21:38:23Z

I suspect that this needs updates to reflect the type changes with 52.02; if you look at the changes made for 52.02 compatibility you will note that I had to make adjustments to code in script-manager.lua and I suspect parallel changes need to be made here.

Yep, we no longer need to pass .value anymore. I'll do a final pass tomorrow to make sure none of the changes have affected script behaviour.

ab9rf · 2025-07-25T21:53:37Z

This is now working for me in testing in arena mode.

ab9rf · 2025-07-25T22:08:41Z

this looks ready to merge to me, but i'll wait if you want to give it one more pass first

i want to get a release out soon for the fix to mod manager but i'd like to slide this in with that release if i can

ong-yinggao98 · 2025-07-26T10:04:51Z

this looks ready to merge to me, but i'll wait if you want to give it one more pass first

i want to get a release out soon for the fix to mod manager but i'd like to slide this in with that release if i can

I've just finished testing, thanks!

ong-yinggao98 added 2 commits July 13, 2025 03:25

Add flexibility for changing vanilla module versions

df9d85a

Add comments

3b49b24

ong-yinggao98 added 2 commits July 13, 2025 16:29

Edit vanilla mod identification logic

cadf06b

Add support for arena mode

f194ffd

ong-yinggao98 added 2 commits July 13, 2025 18:12

Add notifications for updated mods

d6e1254

Update changelog

a292f12

ong-yinggao98 changed the title ~~Edits to the mod manager~~ gui/mod-manager version compatibility Jul 13, 2025

ong-yinggao98 changed the title ~~gui/mod-manager version compatibility~~ gui/mod-manager improvements Jul 13, 2025

Refactor warning logic

90d437e

ab9rf reviewed Jul 13, 2025

View reviewed changes

gui/mod-manager.lua Outdated Show resolved Hide resolved

Remove excess locals assignment

3655e85

ong-yinggao98 marked this pull request as ready for review July 13, 2025 15:13

ong-yinggao98 added 3 commits July 13, 2025 23:22

Add missing local assignment

3612f48

Add missing changelog entry

71c5b15

Fix possible fallthrough in swap_modlist

16ce689

SilasD reviewed Jul 25, 2025

View reviewed changes

gui/mod-manager.lua Outdated Show resolved Hide resolved

SilasD reviewed Jul 25, 2025

View reviewed changes

Update gui/mod-manager.lua

821fef9

Co-authored-by: SilasD <[email protected]>

ong-yinggao98 and others added 6 commits July 26, 2025 04:30

Update gui/mod-manager.lua

466c058

Co-authored-by: SilasD <[email protected]>

Update gui/mod-manager.lua

077e3c9

Co-authored-by: SilasD <[email protected]>

Update gui/mod-manager.lua

0fb8e3a

Co-authored-by: SilasD <[email protected]>

Update gui/mod-manager.lua

7127f4b

Co-authored-by: SilasD <[email protected]>

Update gui/mod-manager.lua

fbb2ae0

Co-authored-by: SilasD <[email protected]>

Apply suggestions from code review

ef7986a

Co-authored-by: SilasD <[email protected]>

Remove comment

68f87f6

Update for 52.02 paths

72b13db

ab9rf approved these changes Jul 25, 2025

View reviewed changes

Edit docstrings

63a64fd

ab9rf merged commit ca3ab76 into DFHack:master Jul 26, 2025
10 checks passed

ong-yinggao98 deleted the modmanager-vanilla branch July 26, 2025 16:44

ab9rf added a commit that referenced this pull request Jul 26, 2025

correct changelog for #1481

3cec9d6

gui/mod-manager improvements #1481

gui/mod-manager improvements #1481

Uh oh!

Conversation

ong-yinggao98 commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ab9rf commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ong-yinggao98 commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SilasD commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ab9rf commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ong-yinggao98 commented Jul 13, 2025

Uh oh!

Uh oh!

ab9rf commented Jul 25, 2025

Uh oh!

SilasD commented Jul 25, 2025

Uh oh!

Uh oh!

lethosor commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SilasD Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

lethosor Jul 25, 2025

Choose a reason for hiding this comment

Uh oh!

ab9rf commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lethosor commented Jul 25, 2025

Uh oh!

ong-yinggao98 commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ab9rf commented Jul 25, 2025

Uh oh!

ong-yinggao98 commented Jul 25, 2025

Uh oh!

ab9rf commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ab9rf commented Jul 25, 2025

Uh oh!

ong-yinggao98 commented Jul 25, 2025

Uh oh!

ab9rf commented Jul 25, 2025

Uh oh!

ab9rf commented Jul 25, 2025

Uh oh!

ong-yinggao98 commented Jul 26, 2025

Uh oh!

Uh oh!

Uh oh!

`gui/mod-manager` improvements #1481

`gui/mod-manager` improvements #1481

ong-yinggao98 commented Jul 12, 2025 •

edited

Loading

ab9rf commented Jul 12, 2025 •

edited

Loading

ong-yinggao98 commented Jul 12, 2025 •

edited

Loading

SilasD commented Jul 12, 2025 •

edited

Loading

ab9rf commented Jul 13, 2025 •

edited

Loading

lethosor commented Jul 25, 2025 •

edited

Loading

ab9rf commented Jul 25, 2025 •

edited

Loading

ong-yinggao98 commented Jul 25, 2025 •

edited

Loading

ab9rf commented Jul 25, 2025 •

edited

Loading