Substantially increase lump name character limit >8

[Nash]

With regards to what Graf said about sprites, I think that was in response to one of my posts which I deleted after I realized that what's being developed here was irrelevant to what I asked in my post, but he somehow saw the post and replied to it after I deleted it.

[Graf Zahl]

If you'd rather I use Fstring, that's fine, I was just taking a more incremental approach that would make it easier to migrate to something better later.

I'm still not convinced that it would have helped in the end. The only way to change some buffer's size is to find all places in the code where it is used and adjust them. Changing how these are accessed doesn't make it any easier or any safer.

Nevertheless, if you want to work on this I won't complain. Just do us all one favor: Please don't do this on a GZDoom clone. I'd have to merge it back manually in the end. Better work on a direct clone of the ZDoom repo. In that case I could merge it in directly.

[binarycrusader]

If you'd rather I use Fstring, that's fine, I was just taking a more incremental approach that would make it easier to migrate to something better later.

I'm still not convinced that it would have helped in the end. The only way to change some buffer's size is to find all places in the code where it is used and adjust them. Changing how these are accessed doesn't make it any easier or any safer.

I realise that, I was just trying to do things incrementally. I'm aware of the pain of having to find all the places where it is used and adjust them as well. I've spent many hours discovering those rules the hard way.

Based on the commits you've recently made to the zdoom repository, I'll take the same approach in other places. I'll try to do this in smaller chunks to make it easier to review.

Nevertheless, if you want to work on this I won't complain. Just do us all one favor: Please don't do this on a GZDoom clone. I'd have to merge it back manually in the end. Better work on a direct clone of the ZDoom repo. In that case I could merge it in directly.

Sure, not a problem. The close linking of the gzdoom / zdoom repositories just made things a little confusing at first

[Edward-san]

Compiling in linux fails, because SDL code is not adapted to the new changes, and also an error for varargs + Fstrings in generic code:

Code: Select all

/home/edward-san/zdoom/trunk/src/sdl/i_main.cpp:177:24: error: no member named 'mapname' in 'FLevelLocals'
                strncpy (name, level.mapname, 8);
                               ~~~~~ ^
1 error generated.
make[2]: *** [src/CMakeFiles/zdoom.dir/sdl/i_main.o] Error 1
/home/edward-san/zdoom/trunk/src/g_mapinfo.cpp:305:60: error: cannot pass non-trivial object of type 'FString' to variadic function; expected type from format string was 'char *' [-Wnon-pod-varargs]
                                mysnprintf (checkstring, countof(checkstring), "%s: ", MapName);
                                                                                ~~     ^~~~~~~
1 error generated.

Also, new warnings are up:

Code: Select all

/home/edward-san/zdoom/trunk/src/wi_stuff.cpp:329:7: warning: variable 'lumpname' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
                if (li != NULL) lumpname = li->EnterPic;
                    ^~~~~~~~~~
/home/edward-san/zdoom/trunk/src/wi_stuff.cpp:337:6: note: uninitialized use occurs here
        if (lumpname == NULL || lumpname[0]==0) 
            ^~~~~~~~
/home/edward-san/zdoom/trunk/src/wi_stuff.cpp:329:3: note: remove the 'if' if its condition is always true
                if (li != NULL) lumpname = li->EnterPic;
                ^~~~~~~~~~~~~~~~
/home/edward-san/zdoom/trunk/src/wi_stuff.cpp:317:22: note: initialize the variable 'lumpname' to silence this warning
        const char *lumpname;
                            ^
                             = NULL
1 warning generated.
/home/edward-san/zdoom/trunk/src/textures/texturemanager.cpp:236:13: warning: comparison between NULL and non-pointer ('int' and NULL) [-Wnull-arithmetic]
                        if (lump != NULL)
                            ~~~~ ^  ~~~~
1 warning generated.

[Graf Zahl]

ok, changed all.

[Edward-san]

Still doesn't compile... it seems the compilers don't accept the FString conversion as argument to the variadic function:

Code: Select all

/home/edward-san/zdoom/trunk/src/sdl/i_main.cpp:175:65: error: cannot pass
      non-trivial object of type 'FString' to variadic function; expected type
      from format string was 'char *' [-Wnon-pod-varargs]
  ...p += snprintf (buffer+p, size-p, "\n\nCurrent map: %s", level.MapName);
                                                        ~~         ^~~~~~~
1 error generated.

/home/edward-san/zdoom/trunk/src/g_mapinfo.cpp:305:60: error: cannot pass
      non-trivial object of type 'FString' to variadic function; expected type
      from format string was 'char *' [-Wnon-pod-varargs]
  ...mysnprintf (checkstring, countof(checkstring), "%s: ", MapName);
                                                     ~~     ^~~~~~~
1 error generated.
make[2]: *** [src/CMakeFiles/zdoom.dir/g_mapinfo.o] Error 1

Adding 'GetChars()' works for me.

[Edward-san]

You forgot to fix the error in g_mapinfo.cpp as indicated above.

[MG_Man]

Is this going to apply for sprites as well? I realize the last letter and number is extremely important, but perhaps the part before that could be extended?

[edward850]

Is this going to apply for sprites as well? I realize the last letter and number is extremely important, but perhaps the part before that could be extended?

Let's see what the rest of the thread has already said.

Sprites are really tied to their 4444-1-1-[1-1] convention, there's no way around it.

Sorry, but the sprite stuff is too rigidly tied to the naming scheme. Changing it may cause unforeseen problems.

For sprites, sorry, no. The sprite naming system is so hardwired to the naming scheme that I wouldn't like to change that.

That's probably a no, then.

[Graf Zahl]

Make that a big, loud 'Never!'
There are things not worth changing and this clearly is one of them.

[Nash]

I just wanted to drop in and say thank you so much for this. Finally less awkwardly-named file resources! I've converted a huge chunk of my project to use longer filenames and everything seems to be working so far... even ACS libraries with long names work - now I'm not sure if long file name libraries worked all along or if it was introduced by this new feature but MAN it's so good to be able to clearly label what each library is because it was starting to get really tedious to manage with the short names! So once again, thank you for this feature.

(Too bad about the sprites... oh well. I'm sure someone would take a look at it in future... :P)

[binarycrusader]

Perhaps I'm confused by how this is supposed to work, but while this did fix texture definitions, it doesn't quite seem to be enough yet.

For example, if I have the following in a map file:

Code: Select all

sidedef
{
sector = 9;
texturemiddle = "MANOR_WALL_COLDSTONE_ARCH";
}

sidedef
{
sector = 1;
texturemiddle = "MANOR_WALL_BOOKSHELF";
}

And then I have this in my textures.txt:

Code: Select all

texture manor_wall_coldstone_arch, 512, 512
{
		XScale 4
		YScale 4
		Patch "patches/p_manor_wall_coldstone_arch.png", 0, 0
}

texture manor_wall_bookshelf, 512, 512
{
		XScale 4
		YScale 4
		Patch pmwbkshf, 0, 0
}

The texture lookup is always going to succeed, so we never fallthrough to the new lumpname logic that has been added.

This is because when parsing the udmf data, the names are going to be truncated to 8 characters. Later on, when SetTexture is called, it's going to match the second texture definition.

As JP noted in his textures.txt:

Texture entries here are necessary to scale textures to non-1:1 sizes,
so that for example a 512x512 can be drawn on a 128x128 wall and appear at
the correct scale.

Can you clarify as to how this was intended to work? Or was this the part that I still needed to make changes for? The truncation is confusing as the udmf specification explicitly states that "Strings have no reasonable length limitations".

It seems like you were expecting the sidedefs above to specify a full pathname to the resource, and since there would be no textures.txt entry, it would find a match that way. But that doesn't work if you need to apply scaling, etc.

[Gez]

Perhaps I'm confused by how this is supposed to work, but while this did fix texture definitions, it doesn't quite seem to be enough yet.

Code: Select all

texture manor_wall_coldstone_arch, 512, 512
{
		XScale 4
		YScale 4
		Patch "patches/p_manor_wall_coldstone_arch.png", 0, 0
}

texture manor_wall_bookshelf, 512, 512
{
		XScale 4
		YScale 4
		Patch pmwbkshf, 0, 0
}

Textures defined in [wiki]TEXTURES[/wiki] are not lump. The TEXTURES lump still can only define textures with an 8-char name. Texture manager and lump manager are two different things.

You can't create long names with TEXTURES.

[binarycrusader]

Textures defined in [wiki]TEXTURES[/wiki] are not lump. The TEXTURES lump still can only define textures with an 8-char name. Texture manager and lump manager are two different things.

You can't create long names with TEXTURES.

Ah, but you would accept patches to remove this limitation for UDMF maps, as long as it doesn't break older doom maps, mods, etc. correct?

[Nash]

Ok so I'm trying to understand this better (I haven't started renaming my map textures to long names yet)...

Does this mean that this won't work:

manor_wall_bookshelf01
manor_wall_bookshelf02

... the engine will only recognize it as MANOR_WA?

(Sorry if this seems like a dumb question; I'm still unsure about the technicalities of this feature and what limits to look out for)