From 2f131db53a17f73314b06abeace142971900a6ac Mon Sep 17 00:00:00 2001 From: Goober5000 Date: Sun, 4 Oct 2026 13:09:42 -0400 Subject: [PATCH] propagate POF load failures through virtual POFs A virtual POF ignored the load status of its base POF, so a base that failed to load (missing, bad version, or rejected after parsing) was still built upon and reported as a successful load. Likewise, every operation that copies from another POF used it even if that POF had failed to load. Return FAIL from read_virtual_model_file when the base POF fails, and have the operations skip, with a warning, any source POF that failed. Co-Authored-By: Claude Opus 5.5 --- code/model/modelread.cpp | 13 +++------ code/model/modelreplace.cpp | 58 ++++++++++++++++++++++++++++--------- code/model/modelreplace.h | 2 +- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/code/model/modelread.cpp b/code/model/modelread.cpp index 5cc24400da..3acf8a430f 100644 --- a/code/model/modelread.cpp +++ b/code/model/modelread.cpp @@ -3088,17 +3088,12 @@ modelread_status read_model_file_no_subsys(polymodel * pm, const char* filename, modelread_status read_model_file(polymodel* pm, const char* filename, ErrorType error_type, model_read_deferred_tasks& deferredTasks, model_parse_depth depth = {}) { - modelread_status status; - //See if this is a modular, virtual pof, and if so, parse it from there - if (read_virtual_model_file(pm, filename, std::move(depth), error_type, deferredTasks)) { - status = modelread_status::SUCCESS_VIRTUAL; - } - else { - status = read_model_file_no_subsys(pm, filename, error_type, deferredTasks); - } + auto virtual_status = read_virtual_model_file(pm, filename, std::move(depth), error_type, deferredTasks); + if (virtual_status) + return *virtual_status; - return status; + return read_model_file_no_subsys(pm, filename, error_type, deferredTasks); } //reads a binary file containing a 3d model diff --git a/code/model/modelreplace.cpp b/code/model/modelreplace.cpp index f29858c1df..47fb0d653a 100644 --- a/code/model/modelreplace.cpp +++ b/code/model/modelreplace.cpp @@ -50,6 +50,7 @@ public: model_read_deferred_tasks _deferred; SCP_set keepTextures{}, keepGlowbanks{}, keepSM{}; bool needs_emplace = false; + bool _loaded = false; friend class VirtualPOFBuildCache; public: @@ -58,6 +59,7 @@ public: create_family_tree(_pm); if (status == modelread_status::SUCCESS_REAL) needs_emplace = true; + _loaded = (status != modelread_status::FAIL); } //Don't copy, just move @@ -75,6 +77,7 @@ public: inline const model_read_deferred_tasks& deferred() const { return _deferred; } inline const polymodel* pm() const { return _pm; } + inline bool loaded() const { return _loaded; } inline void keepTexture(int texture) { keepTextures.emplace(texture); } inline void keepGlowbank(int gb) { keepGlowbanks.emplace(gb); } inline void keepBSPData(int sm) { keepSM.emplace(sm); } @@ -101,6 +104,16 @@ public: } virtual_pof_build_cache; +// Returns nullptr, with a warning, if the POF could not be loaded +static std::shared_ptr load_source_pof(const SCP_string& pof_name, model_parse_depth& depth, const VirtualPOFDefinition& virtualPof) { + auto pmh = virtual_pof_build_cache(pof_name, depth); + if (!pmh->loaded()) { + Warning(LOCATION, "Could not load POF %s for virtual POF %s. Returning original POF.", pof_name.c_str(), virtualPof.name.c_str()); + return nullptr; + } + return pmh; +} + // General Functions and external code paths bool model_exists(const SCP_string& filename) { @@ -112,28 +125,29 @@ bool model_exists(const SCP_string& filename) { return cf_exists_full(filename.c_str(), CF_TYPE_MODELS); } -bool read_virtual_model_file(polymodel* pm, const SCP_string& filename, model_parse_depth depth, ErrorType error_type, model_read_deferred_tasks& deferredTasks) { +std::optional read_virtual_model_file(polymodel* pm, const SCP_string& filename, model_parse_depth depth, ErrorType error_type, model_read_deferred_tasks& deferredTasks) { auto virtual_pof_it = virtual_pofs.find(filename); //We don't have a virtual pof if(virtual_pof_it == virtual_pofs.end()) - return false; + return std::nullopt; //We have one, but we're already past it and are processing whatever it overwrote int& depthLocal = depth[filename]; if ((int)virtual_pof_it->second.size() <= depthLocal) - return false; + return std::nullopt; const auto& virtual_pof = virtual_pof_it->second[depthLocal]; depthLocal++; - read_model_file(pm, virtual_pof.basePOF.c_str(), error_type, deferredTasks, depth); + if (read_model_file(pm, virtual_pof.basePOF.c_str(), error_type, deferredTasks, depth) == modelread_status::FAIL) + return modelread_status::FAIL; for (const auto& operation : virtual_pof.operationList) operation->process(pm, deferredTasks, depth, virtual_pof); - return true; + return modelread_status::SUCCESS_VIRTUAL; } void virtual_pof_purge_cache() { @@ -349,7 +363,9 @@ VirtualPOFOperationAddSubmodel::VirtualPOFOperationAddSubmodel() { } void VirtualPOFOperationAddSubmodel::process(polymodel* pm, model_read_deferred_tasks& deferredTasks, model_parse_depth depth, const VirtualPOFDefinition& virtualPof) const { - auto appendingPMholder = virtual_pof_build_cache(appendingPOF, depth); + auto appendingPMholder = load_source_pof(appendingPOF, depth, virtualPof); + if (appendingPMholder == nullptr) + return; const model_read_deferred_tasks& appendingSubsys = appendingPMholder->deferred(); const polymodel* appendingPM = appendingPMholder->pm(); @@ -524,7 +540,9 @@ VirtualPOFOperationAddTurret::VirtualPOFOperationAddTurret() { } void VirtualPOFOperationAddTurret::process(polymodel* pm, model_read_deferred_tasks& deferredTasks, model_parse_depth depth, const VirtualPOFDefinition& virtualPof) const { - auto appendingPM = virtual_pof_build_cache(appendingPOF, depth); + auto appendingPM = load_source_pof(appendingPOF, depth, virtualPof); + if (appendingPM == nullptr) + return; SCP_unordered_map replaceSubmodelNo; @@ -588,7 +606,9 @@ VirtualPOFOperationAddEngine::VirtualPOFOperationAddEngine() { } void VirtualPOFOperationAddEngine::process(polymodel* pm, model_read_deferred_tasks& deferredTasks, model_parse_depth depth, const VirtualPOFDefinition& virtualPof) const { - auto appendingPM = virtual_pof_build_cache(appendingPOF, depth); + auto appendingPM = load_source_pof(appendingPOF, depth, virtualPof); + if (appendingPM == nullptr) + return; const auto& engineSubsysMap = appendingPM->deferred().engine_subsystems; int engineNumber; @@ -665,7 +685,9 @@ VirtualPOFOperationAddGlowpoint::VirtualPOFOperationAddGlowpoint() { } void VirtualPOFOperationAddGlowpoint::process(polymodel* pm, model_read_deferred_tasks& /*deferredTasks*/, model_parse_depth depth, const VirtualPOFDefinition& virtualPof) const { - auto appendingPM = virtual_pof_build_cache(appendingPOF, depth); + auto appendingPM = load_source_pof(appendingPOF, depth, virtualPof); + if (appendingPM == nullptr) + return; int dest_subobj_no = model_find_submodel_index(pm, renameSubmodel.c_str()); @@ -702,7 +724,9 @@ VirtualPOFOperationAddSpecialSubsystem::VirtualPOFOperationAddSpecialSubsystem() } void VirtualPOFOperationAddSpecialSubsystem::process(polymodel* /*pm*/, model_read_deferred_tasks& deferredTasks, model_parse_depth depth, const VirtualPOFDefinition& virtualPof) const { - auto appendingPM = virtual_pof_build_cache(appendingPOF, depth); + auto appendingPM = load_source_pof(appendingPOF, depth, virtualPof); + if (appendingPM == nullptr) + return; const auto& subsystems = appendingPM->deferred().model_subsystems; auto it = subsystems.find(sourceSubsystem); @@ -744,7 +768,9 @@ VirtualPOFOperationAddWeapons::VirtualPOFOperationAddWeapons() { } void VirtualPOFOperationAddWeapons::process(polymodel* pm, model_read_deferred_tasks& /*deferredTasks*/, model_parse_depth depth, const VirtualPOFDefinition& virtualPof) const { - auto appendingPM = virtual_pof_build_cache(appendingPOF, depth); + auto appendingPM = load_source_pof(appendingPOF, depth, virtualPof); + if (appendingPM == nullptr) + return; auto& banks = primary ? pm->gun_banks : pm->missile_banks; int& n_banks = primary ? pm->n_guns : pm->n_missiles; @@ -812,7 +838,10 @@ VirtualPOFOperationAddDockPoint::VirtualPOFOperationAddDockPoint() { } void VirtualPOFOperationAddDockPoint::process(polymodel* pm, model_read_deferred_tasks& /*deferredTasks*/, model_parse_depth depth, const VirtualPOFDefinition& virtualPof) const { - const polymodel* appendingPM = virtual_pof_build_cache(appendingPOF, depth)->pm(); + auto appendingPMholder = load_source_pof(appendingPOF, depth, virtualPof); + if (appendingPMholder == nullptr) + return; + const polymodel* appendingPM = appendingPMholder->pm(); int dockpoint = model_find_dock_name_index(appendingPM, sourcedock.c_str()); if (dockpoint < 0) { @@ -912,7 +941,10 @@ VirtualPOFOperationAddPath::VirtualPOFOperationAddPath() { } void VirtualPOFOperationAddPath::process(polymodel* pm, model_read_deferred_tasks& /*deferredTasks*/, model_parse_depth depth, const VirtualPOFDefinition& virtualPof) const { - const polymodel* appendingPM = virtual_pof_build_cache(appendingPOF, depth)->pm(); + auto appendingPMholder = load_source_pof(appendingPOF, depth, virtualPof); + if (appendingPMholder == nullptr) + return; + const polymodel* appendingPM = appendingPMholder->pm(); int sourcePathNr = -1; for (int i = 0; i < appendingPM->n_paths; i++) { diff --git a/code/model/modelreplace.h b/code/model/modelreplace.h index 6535d8783a..f9d0c7bfb5 100644 --- a/code/model/modelreplace.h +++ b/code/model/modelreplace.h @@ -8,7 +8,7 @@ #include bool model_exists(const SCP_string& filename); -bool read_virtual_model_file(polymodel* pm, const SCP_string& filename, model_parse_depth depth, ErrorType error_type, model_read_deferred_tasks& deferredTasks); +std::optional read_virtual_model_file(polymodel* pm, const SCP_string& filename, model_parse_depth depth, ErrorType error_type, model_read_deferred_tasks& deferredTasks); void virtual_pof_purge_cache(); void virtual_pof_init();