Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove references to pages that don't exist (i. e. have no .prp file) #1366

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dgelessus
Copy link
Contributor

And add an hsAssert instead of just a log message when trying to load a nonexistant page, so that such issues will be easier to notice in the future, at least in debug builds.

The Python changes are the most relevant bit - they fix all scripts that tried to actively load nonexistant pages. The .age changes are less important in practice, because as far as I can tell, the engine doesn't do anything with pages that are just listed in the .age file and never explicitly loaded somehow.

I'm also a bit suspicious that so many nonexistant pages were listed in the .age files, including pages that never existed in any public version of Uru (as far as I can tell). I'm assuming that they are safe to remove, but perhaps these entries have some purpose that I'm not aware of...

@dpogue
Copy link
Member

dpogue commented Mar 23, 2023

I'm reasonably sure that these entries weren't in the .age files for PotS or MOUL... my guess is a bunch of development files were used when open-sourcing moul-scripts, or something to that effect

@@ -55,7 +55,7 @@ void plClientMsg::AddRoomLoc(const plLocation& loc)
if (loc.IsValid())
fRoomLocs.emplace_back(loc);
else
hsStatusMessage("Trying to load an invalid room, ignoring");
hsAssert(false, "Trying to load an invalid room");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The challenge with this is that it only fires in debug builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, we could make it a hard error everywhere. I can't think of a good reason why you'd want to silently ignore invalid page loads, but perhaps there are weird cases that I don't know about.

Or just leave the original log message in place, in addition to the assert. Though I feel like the log message on its own isn't a good tool for uncovering these issues, considering that it has appeared on every game start for years without anybody paying attention to it...

@Hoikas
Copy link
Member

Hoikas commented May 7, 2023

I've been sitting on this for awhile, thinking that something isn't quite right with this idea. I finally remembered what's not good about this today. Artists who use the Max plugin have to write their .age files by hand, so it's possible that the .age file that an artist hands off has PRPs, potentially for future content, listed that they don't actually distribute yet. I'm not sure how I feel about requiring cutting down the .age file manually in that case. I'd like to hear from others before proceeding.

@Hazado
Copy link
Contributor

Hazado commented May 7, 2023

I know that prpshop wont load the prps if the first prp listed in the age file doesnt exist.

@Hoikas
Copy link
Member

Hoikas commented May 7, 2023

That sounds like a good bug to file on H-uru/PlasmaShop 😉

@dgelessus
Copy link
Contributor Author

Looking at this again, perhaps I'll revert my .age file changes here. The nonexistant entries don't cause any problems, so if they're potentially useful during development, we can just leave them in. The Python changes are the important bit, because they fix all the cases where the game tried to actually load nonexistant pages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants