[PULL REQUEST] Export FBoundingBox to ZScript

Post a reply

Smilies
:D :) :( :o :shock: :? 8-) :lol: :x :P :oops: :cry: :evil: :twisted: :roll: :wink: :geek: :ugeek: :!: :?: :idea: :arrow: :| :mrgreen: :3: :wub: >:( :blergh:
View more smilies

BBCode is OFF
Smilies are ON

Topic review
   

Expand view Topic review: [PULL REQUEST] Export FBoundingBox to ZScript

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by phantombeta » Sun Oct 20, 2019 9:24 am

It seems this thread was left open. I've closed the PR a while ago, so this should be moved to Closed Feature Suggestions. (Maybe labelled something like just "Closed")

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by phantombeta » Wed Dec 26, 2018 6:41 am

I've given up on this PR due to some issues, so this can be closed.

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by phantombeta » Tue Dec 18, 2018 5:27 pm

Just to be sure, are there any issues with this PR?
I'd also like to know if this is really the proper way to do this - I feel like FBoundingBox itself should be exported so it's more useful for future scriptification of actors, but I can't simply do it, since everything creates and destructs it manually.

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by drfrag » Tue Dec 11, 2018 12:50 pm

Rachael wrote:and it's not known or likely to cause any other issues
That's precisely the point here, oops i've reverted it anyway. :P
I've noticed myself that the previous "solution" was a bit shitty and now i've added a FDrawInfo* parameter to GLPortal::End.
I don't even know what outer_di is but hey if it works it works. :D
https://github.com/drfrag666/gzdoom/com ... 9ac12ce215

I've noticed that LZDoom is also affected by this shit, there was a similar problem in 3.3.2 and seems something i cherry-picked has made it come back.

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by Rachael » Tue Dec 11, 2018 6:21 am

drfrag wrote:You shouldn't be too worried about that commit after i merged over 400 commits in a couple of days. :)
But it's not the PR, it's a bugfix for the dot in the sky: viewtopic.php?f=7&t=62518

I merged it becouse i wanted to prove myself i was capable of doing it, it was not important. I could have created a test branch but instead i've reverted it for now until it's reviewed. BTW what do you think of those commits?
Actually - if it's a bugfix and it's not known or likely to cause any other issues, it should be unconditionally pushed. My mistake. You probably should revert that revert. :) hehe

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by drfrag » Tue Dec 11, 2018 4:55 am

You shouldn't be too worried about that commit after i merged over 400 commits in a couple of days. :)
But it's not the PR, it's a bugfix for the dot in the sky: viewtopic.php?f=7&t=62518

I merged it becouse i wanted to prove myself i was capable of doing it, it was not important. I could have created a test branch but instead i've reverted it for now until it's reviewed. BTW what do you think of those commits?
@Graf:
What i did (besides using FBoundingBox and adding the Contains methods) was adding the FDrawInfo *di parameter to GLPortal::DrawPortalStencil and use it in the GLPortal::Start calls, in GLPortal::End i called it with a null pointer instead and inside the method i used the old code with STENCILTOP and STENCILBOTTOM indexes if di is null.
I don't even know how stencil works and i'm not sure if that makes much sense but the test wad works well. :)

What happened with 3.5.1 is that i didn't notice some commits in there were not in the main point release until a moment before but those did not introduce bugs (i reviewed them) and i was already aware of the problem with settings controllers but i chose not to revert it for that legacy release.

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by Rachael » Mon Dec 10, 2018 6:52 am

Be careful about committing directly to your legacy branch without creating a test branch, if indeed you merged this PR. That's going to create another 3.5.1-like situation where you had some unreleased 3.6.0 features in your release build.

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by drfrag » Mon Dec 10, 2018 5:42 am

And surprisingly i've managed to merge it and apparently it's working now but i don't know why, may be it's just pure luck. :)
AllocVertices was in FDrawInfo in that revision. I tested with thebox.wad.

I've reverted it for now, the following two commits need reviewing.
I applied "- fixed: Horizon portals must be drawn in the context of their containing drawinfo" in the middle of them.
https://github.com/drfrag666/gzdoom/com ... 07cb6ed116
https://github.com/drfrag666/gzdoom/com ... 287c0f70f0

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by drfrag » Sun Dec 09, 2018 3:07 pm

@Graf Zahl:
And still "changed the stencil cap drawer to only cover the area which is actually used by the portal" cannot be merged, i've added the Contains methods to FBoundingBox (it had only inRange which requires a line and i need the camera position). But i also need AllocVertices in DrawPortalStencil and it's in HWDrawInfo not in FFlatVertexBuffer.

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by phantombeta » Sat Dec 08, 2018 1:12 pm

I've made the requested changes. Should be ready to merge.

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by drfrag » Sat Dec 08, 2018 12:50 pm

Yeah, i've noticed i've messed up things quite a bit. I thought it was a newer bounding box based on BoundingRect. :oops:
I'll try to merge that commit anyway.

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by phantombeta » Sat Dec 08, 2018 12:38 pm

I'm wondering something: To actually be able to export it, I had to define a new class named "DBoundingBox", which inherits from both FBoundingBox and DObject. This is due to the fact that FBoundingBox doesn't inherit from DObject.
Before doing that, I actually tried to export FBoundingBox itself by making it inherit from DObject, but since it's created and destructed everywhere, this just made GZDoom complain that it was being destroyed outside the GC.
Is this the correct way to export it, or is there some way to export FBoundingBox itself?

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by Graf Zahl » Sat Dec 08, 2018 12:35 pm

The PR is already based on FBoundingBox, not FBoundingRect. I should probably merge those two together, they are a bit redundant.

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by drfrag » Sat Dec 08, 2018 11:57 am

Thanks, you've used the newer BoundingRect struct. I'll try to implement it using the FBoundingBox class instead.

Re: [PULL REQUEST] Export FBoundingBox to ZScript

by Graf Zahl » Sat Dec 08, 2018 6:10 am

In that case you'll have to manually merge the parts you need. What do you need from that commit anyway? FBoundingBox is old stuff.

But don't be surprised. It will get increasingly difficult to keep the code base up to date. We all knew that the vintage build was merely a stopgap measure to delay the inevitable. If we can get one or two more releases out it would be long enough to build a graceful exit strategy. I guess once the texture manager rework I started this week is done there will be no way back, because that will inevitably dig deep into the GL renderer as well when I seriously start fixing all the deficiencies in there which are owed to a container that was singlemindedly written for the software renderer.

I'll be putting this one on hold until you have sorted the bounding box out.

Top