Skip to content

Conversation

@sirjuddington
Copy link
Owner

This is an adaptation of #966 to work with the current code base. All of the WIPs/issues in @eevee's comment there still apply.

From the small amount of testing I have done so far, it seems to work ok enough for basic, solid 3d floors at least. Swimmable/transparent 3d floors have quite a few bugs on the other hand, not sure if I've introduced them during the adaptation process or if they were there in the old branch.

So yeah this definitely isn't ready for a proper release just yet, and likely needs a bunch of work to get up to scratch. I've added a cvar to enable/disable 3d floors processing so it could potentially be released as an 'experimental' feature that is off by default or something.

And of course if anyone wants to have a go at fixing all the issues with it and/or adding the missing features, feel free :P

Some unrelated changes were ignored but hopefully I haven't missed anything important

Seems to work ok for the most part, at least for basic, solid 3d floors (swimmable/transparent ones are another matter, plenty of bugs there)

I've also added the cvar 'map_process_3d_floors' which will enable/disable 3d floor processing
@FlykeSpice
Copy link
Contributor

FlykeSpice commented May 12, 2022

EDIT: Fixed with below commit b6c959a

I don't know why but I'm getting crash every time I try to switch to 3D mode in the map editor. (gdb log below)

Thread 1 "slade" received signal SIGSEGV, Segmentation fault.~
0x0000555555de66e1 in slade::MapRenderer3D::renderSky (this=0x555569b85530) at slade/src/MapEditor/Renderer/MapRenderer3D.cpp:911
911 if ((flats_[a]->flags & SKY) == 0)
(gdb) bt
#0 0x0000555555de66e1 in slade::MapRenderer3D::renderSky() (this=0x555569b85530) at slade/src/MapEditor/Renderer/MapRenderer3D.cpp:911
#1 0x0000555555df2f98 in slade::MapRenderer3D::renderMap() (this=this@entry=0x555569b85530) at slade/src/MapEditor/Renderer/MapRenderer3D.cpp:678
#2 0x0000555555e0aa80 in slade::mapeditor::Renderer::drawMap3d() (this=0x555569b853f0) at slade/src/MapEditor/Renderer/Renderer.cpp:1204
#3 0x0000555555e0c945 in slade::mapeditor::Renderer::draw() (this=0x555569b853f0) at slade/src/MapEditor/Renderer/Renderer.cpp:1239
#4 0x0000555555e2ee33 in slade::MapCanvas::draw() (this=0x55556969a400) at slade/src/MapEditor/UI/MapCanvas.cpp:114

…ray (#1376)

The array was being used uninitialized causing random crashes
…the control sector (#1381)

It was actually being forwarded to the target sector from the control sector, causing ambiguous issues like when you changing the 3d floor height/textures you were changing the ones in the sector it was over instead.
@OrdinaryMagician
Copy link

Any chance of this ever making it in?

@sirjuddington
Copy link
Owner Author

Any chance of this ever making it in?

Hmm well I doubt it will be finished (ie. all bugs fixed) any time soon, but as I mentioned in the description I guess it could be added as-is but disabled by default, with a warning that it's incomplete/buggy when enabling it.

Repository owner deleted a comment from sonarqubecloud bot Aug 28, 2023
@wallabra
Copy link
Contributor

wallabra commented Jan 3, 2024

Would it be better to have this as a draft?

[...]I guess it could be added as-is but disabled by default, with a warning that it's incomplete/buggy when enabling it.

I think having it as an opt-in beta feature in the main branch builds would make it more accessible for testers, and potentially energize the bug finding and fixing process.

@sirjuddington sirjuddington merged commit 115377d into master Jan 5, 2024
@Pedro-Beirao
Copy link
Contributor

Since this PR, the 3d mode crashes on macOS. Even with 3d floors disabled

I'll try to find the problem tomorrow, but here's the crash if you know how to fix

slade(31963,0x1e3c81300) malloc: Heap corruption detected, free list is damaged at 0x600002c3be50
*** Incorrect guard value: 105553134610992
slade(31963,0x1e3c81300) malloc: *** set a breakpoint in malloc_error_break to debug

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.

6 participants