[PULL REQUEST] Export FBoundingBox to ZScript

Remember, just because you request it, that doesn't mean you'll get it.

Moderator: Developers

[PULL REQUEST] Export FBoundingBox to ZScript

Postby phantombeta » Thu Dec 06, 2018 10:32 am

PR link

This PR exports FBoundingBox to ZScript. This is useful for custom movement code, for example.
This example I've given is actually my use case - I'm making my own chase direction selection function based on P_NewChaseDir, and the drop-off checking code uses FBoundingBox's InRange and BoxOnLineSide functions. While it's pretty much just math - meaning it can all be done in ZScript - it'd be nice to have it all available in GZDoom itself.
User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: The United Soviet Socialist Dictatorship of Hueland
Discord: phantombeta#2461
Twitch ID: phantombeta_

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby phantombeta » Sat Dec 08, 2018 1:38 am

After quite a lot of testing, this seems to be fine. It can be safely merged now.
User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: The United Soviet Socialist Dictatorship of Hueland
Discord: phantombeta#2461
Twitch ID: phantombeta_

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby Graf Zahl » Sat Dec 08, 2018 5:43 am

Like your other PR, please consider direct native functions when adding VM exports.

I can already tell that in the future I won't merge PRs which export functions and not provide these. And also, please try to keep the VM exports out of the main code. It was a major hassle to go through the main files once and move the majority of these to the side where they don't get in the way.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby drfrag » Sat Dec 08, 2018 6:38 am

And this cannot be merged into the legacy build, i could not merge "- changed the stencil cap drawer to only cover the area which is actually used by the portal." That uses sections and i don't know how to implement the bounding box in a different way with existing old code. I really need some hints on how to do it/some help. :(
User avatar
drfrag
I.R developer, I.R smart
Vintage GZDoom Developer
 
Joined: 23 Apr 2004
Location: Spain

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby Graf Zahl » Sat Dec 08, 2018 7: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.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby drfrag » Sat Dec 08, 2018 12:57 pm

Thanks, you've used the newer BoundingRect struct. I'll try to implement it using the FBoundingBox class instead.
User avatar
drfrag
I.R developer, I.R smart
Vintage GZDoom Developer
 
Joined: 23 Apr 2004
Location: Spain

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby Graf Zahl » Sat Dec 08, 2018 1:35 pm

The PR is already based on FBoundingBox, not FBoundingRect. I should probably merge those two together, they are a bit redundant.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby phantombeta » Sat Dec 08, 2018 1: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?
User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: The United Soviet Socialist Dictatorship of Hueland
Discord: phantombeta#2461
Twitch ID: phantombeta_

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby drfrag » Sat Dec 08, 2018 1: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.
User avatar
drfrag
I.R developer, I.R smart
Vintage GZDoom Developer
 
Joined: 23 Apr 2004
Location: Spain

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby phantombeta » Sat Dec 08, 2018 2:12 pm

I've made the requested changes. Should be ready to merge.
User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: The United Soviet Socialist Dictatorship of Hueland
Discord: phantombeta#2461
Twitch ID: phantombeta_

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby drfrag » Sun Dec 09, 2018 4: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.
User avatar
drfrag
I.R developer, I.R smart
Vintage GZDoom Developer
 
Joined: 23 Apr 2004
Location: Spain

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby drfrag » Mon Dec 10, 2018 6: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
User avatar
drfrag
I.R developer, I.R smart
Vintage GZDoom Developer
 
Joined: 23 Apr 2004
Location: Spain

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby Rachael » Mon Dec 10, 2018 7: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.
User avatar
Rachael
QZDoom + Webmaster
 
Joined: 13 Jan 2004

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby drfrag » Tue Dec 11, 2018 5: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.
Last edited by drfrag on Tue Dec 11, 2018 1:51 pm, edited 1 time in total.
User avatar
drfrag
I.R developer, I.R smart
Vintage GZDoom Developer
 
Joined: 23 Apr 2004
Location: Spain

Re: [PULL REQUEST] Export FBoundingBox to ZScript

Postby Rachael » Tue Dec 11, 2018 7: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
User avatar
Rachael
QZDoom + Webmaster
 
Joined: 13 Jan 2004

Next

Return to Feature Suggestions

Who is online

Users browsing this forum: Bing [Bot] and 2 guests