Hyphens in resource names. Bug 566191

[nux]

https://bugs.launchpad.net/armagetronad/+bug/566191

This bug has been open for quite a while, marked as critical and confirmed, but yet no work has been done about it. I haven been playing around with resources with hyphen and couldnt make the client crash or anything like that. From the dedicated server, you get the warning printed in STDOUT. From the client, you get a warning on the ingame console only.

The warning im getting in the (almost) latest alpha is

Warning in bool st_checkName(const tString&) in ../../src/resource/tResourceManager.cpp:350:
Resource names must not start with a dot or contain slashes or minus signs

which doesnt seem to be a problem. The resource loads without errors.

Triage options:
- Fix warning to be a real warning (and not break the game)
- Permit hyphens in resource names (103 in the official repo; compare to 11 with hyphen in version)
- Break compatibility with old resources using hyphens in the name properly (requires further discussion and migration plan?)

My proposal would be to loosely allow hyphens in the name, issuing a warning only on the server STDERR (so it can be logged if server is started by something like `armagetron-dedicated 2> error.log`) and displaying a message to the clients console mentioning this issue in case theres a compatibility issue; maybe drop the programming talk to make it more clear to users. Hyphens cant be allowed in resource names because they act as a delimiter for the version, if we do, we would need another delimiter, which would have the same problem. I dont think breaking compatibility is necessary though, but i guess its rather a project management decision.

[Z-Man]

- Fix warning to be a real warning (and not break the game)

I think that is what we already did. Luke's bug reports can be a bit sparse, the whole process resulted in a crash, IIRC, or things not working in some other fatal fashion. That has been fixed, just the bug entry hasn't been updated. It's still open because there is a bit of an iffiness about the situation of having resources out there in the wild in our repository that are technically considered illegal, and that by development versions only.

issuing a warning only on the server STDERR (so it can be logged if server is started by something like `armagetron-dedicated 2> error.log`)

That's a splendid idea. A patch that generally displays warnings issued the same way od STDERR on both server and release client (so the client usually stays silent on the standard channel, just errors and warnings are logged) will be accepted.

maybe drop the programming talk

Not sure what you mean there. The file location notice?

[nux]

- Fix warning to be a real warning (and not break the game)

I think that is what we already did. Luke's bug reports can be a bit sparse, the whole process resulted in a crash, IIRC, or things not working in some other fatal fashion. That has been fixed, just the bug entry hasn't been updated. It's still open because there is a bit of an iffiness about the situation of having resources out there in the wild in our repository that are technically considered illegal, and that by development versions only.

I kinda figured, since i couldnt manage it to crash. In my eyes, that particular bug should be closed, but a new one open depending on which route we'll go; there is some work to do for each one of them. Also, i wanted to see how much this whole hyphen thing affects us, but i couldnt find a way to download the whole resource repository to run a simple find|grep. I was able to gather this from my local data though:

n@ubuntu:~$ find .armagetronad/resource/ | grep -e "-.*-" | wc -l
12

issuing a warning only on the server STDERR (so it can be logged if server is started by something like `armagetron-dedicated 2> error.log`)

That's a splendid idea. A patch that generally displays warnings issued the same way od STDERR on both server and release client (so the client usually stays silent on the standard channel, just errors and warnings are logged) will be accepted.

I was just talking about that in the irc channel with a few people. Im going to try directing all errors to STDERR and see if i can manage to do this before alpha becomes stable. I believe its gonna take a while to check everything though.

n@ubuntu:~$ grep -rni "std::cerr" arma/0.4/src | wc -l
108
n@ubuntu:~$ grep -rni "std::cout" arma/0.4/src | wc -l
228

maybe drop the programming talk

Not sure what you mean there. The file location notice?

I mean the first part, as it makes it look like theres something wrong with the client, when its all the "server's" fault for pointing to that resource: Warning in bool st_checkName(const tString&) in ../../src/resource/tResourceManager.cpp:350: