From 0b66465f33f287f7a94a825d1bb7f24f56970c10 Mon Sep 17 00:00:00 2001 From: sfan5 Date: Tue, 26 Aug 2025 15:22:36 +0200 Subject: [PATCH] Adjust Server::dynamicAddMedia() and related parts a bit --- doc/lua_api.md | 16 +++---- src/script/lua_api/l_server.cpp | 1 + src/server.cpp | 81 ++++++++++++++++++++------------- src/server.h | 3 ++ 4 files changed, 60 insertions(+), 41 deletions(-) diff --git a/doc/lua_api.md b/doc/lua_api.md index 7182bedc48..eb71151fe9 100644 --- a/doc/lua_api.md +++ b/doc/lua_api.md @@ -7379,7 +7379,7 @@ Server it will not be cached on the client (optional, default false) * Exactly one of the parameters marked [*] must be specified. * `callback`: function with arguments `name`, which is a player name - * Pushes the specified media file to client(s). (details below) + * Pushes the specified media file to client(s) as detailed below. The file must be a supported image, sound or model format. Dynamically added media is not persisted between server restarts. * Returns false on error, true if the request was accepted @@ -7388,19 +7388,17 @@ Server * Details/Notes: * If `ephemeral`=false and `to_player` is unset the file is added to the media sent to clients on startup, this means the media will appear even on - old clients if they rejoin the server. + old clients (<5.3.0) if they rejoin the server. * If `ephemeral`=false the file must not be modified, deleted, moved or - renamed after calling this function. - * Regardless of any use of `ephemeral`, adding media files with the same - name twice is not possible/guaranteed to work. An exception to this is the - use of `to_player` to send the same, already existent file to multiple - chosen players. + renamed after calling this function. This is allowed otherwise. + * Adding media files with the same name twice is not possible. + An exception to this is the use of `to_player` to send the same, + already existent file to multiple chosen players (`ephemeral`=false only). * You can also call this at startup time. In that case `callback` MUST be `nil` and you cannot use `ephemeral` or `to_player`, as these logically do not make sense. * Clients will attempt to fetch files added this way via remote media, - this can make transfer of bigger files painless (if set up). Nevertheless - it is advised not to use dynamic media for big media files. + this can make transfer of bigger files painless (if set up). IPC --- diff --git a/src/script/lua_api/l_server.cpp b/src/script/lua_api/l_server.cpp index 3e9b8b5c62..8b3f19f405 100644 --- a/src/script/lua_api/l_server.cpp +++ b/src/script/lua_api/l_server.cpp @@ -554,6 +554,7 @@ int ModApiServer::l_dynamic_add_media(lua_State *L) } else { tmp = readParam(L, 1); args.filepath = tmp; + log_deprecated(L, "Deprecated call to core.dynamic_add_media() with string argument", 1, true); } if (at_startup) { if (!lua_isnoneornil(L, 2)) diff --git a/src/server.cpp b/src/server.cpp index 139dc0e196..ced5c43555 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -2764,9 +2764,8 @@ void Server::sendRequestedMedia(session_t peer_id, } const auto &m = it->second; - // no_announce <=> usually ephemeral dynamic media, which may - // have duplicate filenames. So we can't check it. - if (!m.no_announce) { + // Ephemeral dynamic media may have duplicate filenames. So we can't check it. + if (!m.ephemeral) { if (!client->markMediaSent(name)) { warningstream << "Server::sendRequestedMedia(): Client has " "requested \"" << name << "\" before, not sending it again." @@ -2834,31 +2833,41 @@ void Server::sendRequestedMedia(session_t peer_id, } } +namespace { + // unordered_map erase_if is only C++20 + template + void erase_if(C &container, F predicate) { + for (auto it = container.begin(); it != container.end(); ) { + if (predicate(*it)) + it = container.erase(it); + else + ++it; + } + } +} + void Server::stepPendingDynMediaCallbacks(float dtime) { EnvAutoLock lock(this); - for (auto it = m_pending_dyn_media.begin(); it != m_pending_dyn_media.end();) { - it->second.expiry_timer -= dtime; - bool del = it->second.waiting_players.empty() || it->second.expiry_timer < 0; + erase_if(m_pending_dyn_media, [&] (decltype(m_pending_dyn_media)::value_type &it) { + auto &[token, state] = it; - if (!del) { - it++; - continue; - } + state.expiry_timer -= dtime; + if (!state.waiting_players.empty() && state.expiry_timer >= 0) + return false; - const auto &name = it->second.filename; + const auto &name = state.filename; if (!name.empty()) { assert(m_media.count(name)); - // if no_announce isn't set we're definitely deleting the wrong file! - sanity_check(m_media[name].no_announce); + sanity_check(m_media[name].ephemeral); fs::DeleteSingleFileOrEmptyDirectory(m_media[name].path); m_media.erase(name); } - getScriptIface()->freeDynamicMediaCallback(it->first); - it = m_pending_dyn_media.erase(it); - } + getScriptIface()->freeDynamicMediaCallback(token); + return true; + }); } void Server::SendMinimapModes(session_t peer_id, @@ -3677,11 +3686,17 @@ namespace { bool Server::dynamicAddMedia(const DynamicMediaArgs &a) { - std::string filename = a.filename; - std::string filepath; + if (!m_env && (!a.to_player.empty() || a.ephemeral)) { + errorstream << "Server: " + "adding ephemeral or player-specific media at startup is nonsense" + << std::endl; + return false; + } // Deal with file -or- data, as provided // (Note: caller must ensure validity, so sanity_check is okay) + std::string filename = a.filename; + std::string filepath; if (a.filepath) { sanity_check(!a.data); filepath = *a.filepath; @@ -3703,29 +3718,30 @@ bool Server::dynamicAddMedia(const DynamicMediaArgs &a) << filepath << std::endl; } - // Do some checks - auto it = m_media.find(filename); - if (it != m_media.end()) { - // Allow the same path to be "added" again in certain conditions - if (a.ephemeral || it->second.path != filepath) { + { + auto it = m_media.find(filename); + if (it == m_media.end()) { + // standard case + } else if (a.ephemeral || it->second.ephemeral || it->second.path != filepath) { + // If the path is the same we can safely allow adding the same file twice. + // Note that we already trust mods to not to modify files after the fact. + // Ephemeral files are excluded too, because currently each + // PendingDynamicMediaCallback "owns" the matching m_media[] entry + // so that would mess up. errorstream << "Server::dynamicAddMedia(): file \"" << filename - << "\" already exists in media cache" << std::endl; + << "\" already exists in media list" << std::endl; return false; } } - if (!m_env && (!a.to_player.empty() || a.ephemeral)) { - errorstream << "Server::dynamicAddMedia(): " - "adding ephemeral or player-specific media at startup is nonsense" - << std::endl; - return false; - } - // Load the file and add it to our media cache std::string filedata, raw_hash; bool ok = addMediaFile(filename, filepath, &filedata, &raw_hash); - if (!ok) + if (!ok) { + if (a.data) // file was temporary + fs::DeleteSingleFileOrEmptyDirectory(filepath); return false; + } assert(!filedata.empty()); const auto &media_it = m_media.find(filename); @@ -3748,6 +3764,7 @@ bool Server::dynamicAddMedia(const DynamicMediaArgs &a) } media_it->second.no_announce = true; + media_it->second.ephemeral = true; // stepPendingDynMediaCallbacks will clean the file up later } else if (a.data) { // data is in a temporary file but not ephemeral, so the cleanup point diff --git a/src/server.h b/src/server.h index 24818c6b0e..007592535b 100644 --- a/src/server.h +++ b/src/server.h @@ -94,6 +94,8 @@ struct MediaInfo std::string sha1_digest; // true = not announced in TOCLIENT_ANNOUNCE_MEDIA (at player join) bool no_announce; + // if true, this is an ephemeral entry. used by dynamic media. + bool ephemeral; // does what it says. used by some cases of dynamic media. bool delete_at_shutdown; @@ -102,6 +104,7 @@ struct MediaInfo path(path_), sha1_digest(sha1_digest_), no_announce(false), + ephemeral(false), delete_at_shutdown(false) { }