[2.8pre-1816] Incorrect Skybox Picker behavior with tag 0

Forum rules
Please don't bump threads here if you have a problem - it will often be forgotten about if you do. Instead, make a new thread here.

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: [2.8pre-1816] Incorrect Skybox Picker behavior with tag 0

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by Graf Zahl » Thu Nov 26, 2015 7:02 am

Technically that makes at least more sense than what GCC does - but both never look at the resulting type, in this case the one that gets returned.
The GCC fix will be filtered down once I update the next time.

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by Edward-san » Thu Nov 26, 2015 5:56 am

graf, can you port the gcc fix to gzdoom?

Also, just for info, clang, too, fails to compile in gzdoom, but with a different message:

Code: Select all

/home/edward-san/zdoom/gzdoom/trunk/src/p_sectors.cpp:809:78: error: 
      conditional expression is ambiguous; 'ASkyViewpoint *' can be converted to
      'TObjPtr<class ASkyViewpoint>' and vice versa
  ...& SECF_NOFLOORSKYBOX)? (ASkyViewpoint*)NULL : level.DefaultSkybox;
                          ^ ~~~~~~~~~~~~~~~~~~~~   ~~~~~~~~~~~~~~~~~~~
/home/edward-san/zdoom/gzdoom/trunk/src/p_sectors.cpp:813:84: error: 
      conditional expression is ambiguous; 'ASkyViewpoint *' can be converted to
      'TObjPtr<class ASkyViewpoint>' and vice versa
  ...& SECF_NOCEILINGSKYBOX)? (ASkyViewpoint*)NULL : level.DefaultSkybox;
                            ^ ~~~~~~~~~~~~~~~~~~~~   ~~~~~~~~~~~~~~~~~~~
2 errors generated.

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by VoidMage » Wed Nov 25, 2015 11:53 am

Well, to be clear, being a file editor is a secondary (or less) function of this program.
I'm talking about internal editor of Midnight Commander - warts and all, in most cases I use it for, it does suffice.

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by Graf Zahl » Wed Nov 25, 2015 11:24 am

Wait, you use an editor that cannot handle different line endings? Those things still exist outside of Notepad? I'm shocked.

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by VoidMage » Wed Nov 25, 2015 10:07 am

OK, I might be slightly biased here, as the editor I most often use, while quite a few steps from a trivial one, doesn't do the automatic line ends conversion, so keeping track of those ^M gets tedious after awhile...

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by Graf Zahl » Wed Nov 25, 2015 10:00 am

Notepad is crap, that's why I do not use it, and do not care if files I create can be read with it.
I'm not talking about Linux/Unix making bad assumptions - I am talking about Linux devs making bad assumptions. That's fully ok for software that's not supposed to be used anywhere else, but for something as 'fixing' line endings something a lot more robust is needed than mutilating line endings on commit/checkout.

I often work on projects where I cannot afford this to have an impact, e.g. Android development on Windows, and yet Git promotes this blanket careless approach to 'fixing' the problem, but ultimately making it even worse.

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by VoidMage » Wed Nov 25, 2015 9:56 am

The way I recall it, Windows had its fair share of bad assumptions too - see notepad in Win9x/XP (it's been fixed quite awhile later).

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by Graf Zahl » Wed Nov 25, 2015 8:20 am

Autocrlf was probably the most idiotic thing Git ever invented, especially how it is implemented.
With SVN this was disabled for good reasons on ZDoom's repo. I have lost count how often a bad combination of Autocrlf, obnoxious editors and bad assumptions which are nearly typical when dealing with Unix-based software (as Git is) managed to mess up things.

I have no idea how those CRLFs constantly creep in again. Visual Studio doesn't do it automatically - I can normally work on a LF-only source base without ever running into issues.

Regarding TObjPtr, sorry, no, that'd be dead wrong. We do not want the TObjPtr semantics on a local variable, that'd be a very bad thing.

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by VoidMage » Wed Nov 25, 2015 8:03 am

Well, a fix is already commited, so it doesn't matter anymore, but I'd fix it slightly differently: change return type of GetSkyBox (and type of skybox in R_Subsector) to TObjPtr<ASkyViewpoint> and remove the cast of NULL (after the former change, gcc is OK with that).

On an unrelated note: you've flipped (probably accidentally) line endings in oal volume commit...
So, food for thought...

Of course, the tricky part is all contributors would need to care about this...

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by Graf Zahl » Wed Nov 25, 2015 6:16 am

GCC sucks. It's truly annoying how anal that compiler is about implicit type conversions. :(

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by VoidMage » Wed Nov 25, 2015 5:58 am

A note: code commited fails with gcc; the error says 'ASkyViewpoint *' and 'TObjPtr<ASkyViewpoint>.' aren't the same type.

Re: [2.8pre-1816] Incorrect Skybox Picker behavior with tag

by Graf Zahl » Wed Nov 25, 2015 4:47 am

Fixed. This actually required adding two new flags because with the default skybox moved to the global level data there was insufficient information to detect this case.

Regarding VS2015, sue you can use that, if you use CMake to generate the project. I'm using VS2013 but the procedure for 2015 is identical.

[2.8pre-1816] Incorrect Skybox Picker behavior with tag 0

by StrikerMan780 » Sat Nov 07, 2015 1:47 pm

Setting the tag for SkyPicker to 0 is supposed to make the sky for that sector use the map default, and not use any SkyBox things. See: [wiki]Classes:SkyPicker[/wiki]

Unfortunately, when ZDoom 2.7 was released, this broke. Now, it uses the first skybox thing with an ID of 0, which is incorrect.

I had once used this ability to have areas with a skybox, and others that use the normal map sky for some interesting effects (such as teleporting to different geological regions), but they no longer work.

This is a shot of the test map showing what it should look like: http://puu.sh/lcZwn/4db4c71dc2.png

As for what it shouldn't look like, here's the test map so you can see for yourself. http://zandronum.com/tracker/file_downl ... 4&type=bug

EDIT: It seems that the Skypicker code is the same between ZDoom 2.6 and 2.8, however, I think someone forgot to compensate for this change in the skybox viewpoint code itself:

2.8's code:

Code: Select all

void ASkyViewpoint::BeginPlay ()
{
	Super::BeginPlay ();

	if (tid == 0 && level.DefaultSkybox == NULL)
	{
		level.DefaultSkybox = this;
	}
}
2.6's code:

Code: Select all

void ASkyViewpoint::BeginPlay ()
{
	Super::BeginPlay ();

	if (tid == 0)
	{
		int i;

		for (i = 0; i <numsectors; i++)
		{
			if (sectors[i].FloorSkyBox == NULL)
			{
				sectors[i].FloorSkyBox = this;
			}
			if (sectors[i].CeilingSkyBox == NULL)
			{
				sectors[i].CeilingSkyBox = this;
			}
		}
	}
}
Is the ZDoom source compatible with Visual Studio 2015: CE? I really should get an environment set up where I can compile this.

Top