DaemonEngine / Daemon

The Dæmon game engine. With some bits of ioq3 and XreaL.
https://unvanquished.net
BSD 3-Clause "New" or "Revised" License
306 stars 60 forks source link

Finding maps requires loading all map paks #557

Closed slipher closed 2 years ago

slipher commented 2 years ago

For various map operations, the engine opens all packages beginning with map-. This is pretty slow and scales poorly, so it would be better if we could avoid it. Different parts of the code use different algorithms to determine what a map is:

One possibility is to enforce that maps/$NAME.bsp must be located in a pak named map-$NAME. This would mean that map commands no longer need to load all packages, and prevent incorrect results from the listmaps client command. However, it would not solve the problems with the UI map list, because it depends on the arena files to determine the map's display name in the dropdown. Assuming we continue to rely on arena files, it seems the only solution there would be some sort of caching.

Gireen commented 2 years ago

That its possible to have multiple maps in a single .dpk is a nice feature but since it not used i can be dropped. Similar with the name in meta/*.arena we could just use $NAME for the map selection and it would not change much. Actually it would probably be more desirable since some maps have broken .arena files and display no name.

slipher commented 2 years ago

Similar with the name in meta/*.arena we could just use $NAME for the map selection and it would not change much.

OK good idea. We can just load the meta/ stuff (arena and levelshot) for a specific map when the user clicks on it.

illwieckz commented 2 years ago

However, it would not solve the problems with the UI map list, because it depends on the arena files to determine the map's display name in the dropdown. Assuming we continue to rely on arena files, it seems the only solution there would be some sort of caching.

At some point I think we may want to just list maps with map- prefix in UI, listing the short name and a dummy picture if nothing else is found. Then look at arena file and fetch longname and image and things like that. At some point the loading of images and other data may be done on the fly.

I thought about this before because I'm worried of how unresponsive our UI is, when we click on the button to open map list, the UI freezes until the list is built. So I thought that maybe with lua or something like that we can make the UI requests the metadata when it's actually needed.

ghost commented 2 years ago

listing the short name and a dummy picture if nothing else is found.

Or while the pak is not loaded yet. Maybe it could be possible to load the meta data in background while still showing the short name (and version?), guessed from the filename.

So I thought that maybe with lua or something like that we can make the UI requests the metadata when it's actually needed.

I experimented a bit with lua and the "start local" GUI yesterday, I think this is possible.

What might do is to provide more daemon<-->RML API, and notably for what you describe, one would need an event to trigger opening and displaying the map's metadata. Something else which would be nice is be able to provide options to the map command, and to merge it with the devmap one, this might make the lua code for this decision useless (and this seems to belong to daemon? I'm not sure, I only started playing with lua and this specific UI yesterday). This would likely require placeholder metadata though, which would both prevent the annoying resize of the box and work around the case where (some?) meta-data is missing (minimap, levelshot, description, version, maybe even licence?). Oh, while I'm at it... the list of maps (and the map command) could use a way to select an older version of the map. Only example which comes to my mind nowadays is parpax: it's layout significantly changed in last iteration and some players might prefer the original one (I would not understand why, but hey, it's possible).

For reference here is what I currently have (not entirely working, layout selection does not work):

<rml>
    <head>
        <link type="text/rcss" href="/ui/shared/basics.rcss" />
        <link type="text/template" href="/ui/shared/window.rml" />
        <style>
            /* TODO: find out why this is a subclass */
            .levelshot {
                width: 100%;
            }
            /* select and selectbox elements are implicit, but their styles must be explicitely written */
            row#maplist dataselect, row#maplist select, input#hostname, input#password {
                width: 25em;
            }
            row#maplist selectbox {
                max-height: 25em;
                overflow-y: auto;
            }

        </style>
        <script>
            function MapMode(document)
                local AsInput = Element.As.ElementFormControlInput
                if AsInput(document:GetElementById("devmode")).checked
                then
                    return 'execForm "devmap $map$ $layout$"'
                end
                return 'execForm "map $map$ $layout$"'
            end
        </script>

    </head>
    <body template="window" id="createserver" onShow='Events.pushevent("buildDS mapList", event)' style="width: 40em; margin: 1em;">
        <h1> Start local/LAN game</h1>
        <form onsubmit='Events.pushevent(MapMode(document)", event)'>
            <row>
                <input type="text" cvar="sv_hostname" class="text" id="hostname"/>
                <h3> Hostname </h3>
            </row>

            <row>
                <input type="text" cvar="g_password" class="text" id="password"/>
                <h3> Password </h3>
            </row>

            <row>
                <input type="checkbox" id="devmode" style="margin-right:1em;"/>
                <h3> Developper mode (enable cheats) </h3>
            </row>

            <row>
                <input type="text" cvar="fs_extrapaks" class="text" id="extrapaks"/>
                <h3> Extra packages to load </h3>
            </row>

            <row>
                <input type="text" class="text" id="layout" name="layout"/>
                <h3> Map building layout to use </h3>
            </row>

            <row id="maplist">
                <dataselect source="mapList.default" fields="mapName" valuefield="mapLoadName" name="map" cvar="ui_dialogCvar1"/>
                <h3> Map </h3>
            </row>

            <input type="submit"> <button> Start </button>  </input>

            <levelshot />

        </form>
    </body>
</rml>
slipher commented 2 years ago

The application is generally single-threaded; you can't load something "in the background". If the disk is slow enough that loading a single pak causes noticeable delay, then clicking around in the list is going to suck no matter what we do

ghost commented 2 years ago

Btw, reading at the PR which motivated this issue: I'm very fine with map-$NAME_$VERSION.dpk being enforced, and that this enforcement would imply that the bsp must have a compatible name. OTOH I'm not a mapper nor trying to port stuff across games. Still, how hard is renaming a file?

slipher commented 2 years ago

I found one named like pushcannon_b3a which can't be done by renaming the archive, since it contains an underscore. But OK, acceptable losses.

illwieckz commented 2 years ago

To me the b3a string is a version so when repackaging I would rename every basename to pushcannon. I prefer strong conventions for multiple reasons, one of them is that strong conventions make easy to make strong assumptions, for example here it may improve speed.

illwieckz commented 2 years ago

I found one named like pushcannon_b3a which can't be done by renaming the archive, since it contains an underscore. But OK, acceptable losses.

So can we merge this?

slipher commented 2 years ago

Fixed by #601.