Nightly: Recent change question

[MacLir]

Code: Select all

r9561 | rubidium | 2007-04-04 14:43:43 +0200 (Wed, 04 Apr 2007) | 3 lines

-Fix (r9561): loading the newgrfs in openttd.cfg was broken.
Note: as the 'full' path is stored of the newgrfs, the newgrfs in the old cfgs are not made default (you have to set them again).

------------------------------------------------------------------------
r9560 | rubidium | 2007-04-04 14:03:10 +0200 (Wed, 04 Apr 2007) | 3 lines

-Codechange: add support for multiple 'base' directories for newgrf searching.
-Codechange: do not add duplicate files to the newgrf list.

In the change.log it states that in r9561 I have to reset my configuration so that my new games start with the newgrfs that I prefer; not a problem. Although once done, OTTD segfaults on a recent save that has those files specified. Removing the list, OTTD can now load that same recent save. I'm not providing the save as its an easily replicable problem.

The first change of r9560 confuses me. I've had two different directories for my newgrfs for awhile now; OTTD listed all of my newgrfs. Which somewhat ties into my problem with the second listed change of r9560. I keep the two sets of newgrfs so that my old saves are still playable, and that all of my new games use updated versions of those newgrfs. It was easy to distinguish the new from the old with just a click. The folder name and file name were both listed for me to see in the newgrf 'add' window. With the 2 directores, it was easy to sort the old from the new. However, at present, its all guess work as to which version I'm using unless the titles of the newgrfs show different names.

With that lead-in, my question is why make those changes? Apart from fixing the config loading, the rest of the changes, seemingly, aren't necessary. Was there a problem I'm not aware of? Truthfully, I'd like to see all but the config loading fix removed so that OTTD regresses to using the old methods of diplaying the folder names with the filename as well as displaying duplicate names. It allows a user to prune their newgrf sets so that duplicate names are only listed for 'necessary' older newgrfs.

Any dev feedback will be appreciated, as I'm much confused and disheartened by these changes...

[Rubidium]

These changes are part of a much larger process of changes that makes it possible to save/load you savegames and place your own grfs in your home directory and have openttd installed somewhere else with some other newgrfs. The problem that then arises is the fact that you need to store the full path of the grfs internally. These paths can become so long (especially under Windows) that you only see "C:\Documents and Settings\MyPrettyLongUsername\..." as filename, which is even more useless than only the name of the file.

Furthermore (I for one) don't want to get bothered by the enormously long path+filenames that will be stored in the savegame when that happens.

On a side-note, I haven't been able to reproduce your segmentation fault. It's probably because it tries to load some incompatible "upgrade" GRF (one with the same GRFID) that misses some stuff.

[MacLir]

Sorry, it wasn't a segmentation fault; it was an assertion failure. (I need to pay more attention to what those error windows tell me.) A screen capture of the error window is attached.

Please correct me if I'm wrong. As I gather from your post, you're saying that when the string for the path plus filename is displayed that its truncated from the left. So, why not process it to filter from the right, so that all the extraneous path information is removed prior to the file's current directory/folder? That would eliminate what you don't like but still allow users some modicum of organization by directories/folders. Also, you could code in this in such a way as to allow any duplicates to still be displayed if their paths differ. At least this is from a string filtering standpoint, as I'm not a coder in the 'programming' sense.

The first point you brought up, the one about 'home' directories/folders, is that in reference to problems on *nix based setups and possibly Vista? If so, shouldn't the read access rights be in place already for the application directory/folder? By just using the newgrf in game, you're not modifing it in any way, so read access should be default and only write access should be a thing for an 'administrator' to allow. Then all you would need to do is process the 'extra' directories/folders that are not under the 'home'. Unless I've missed the point entirely, which I probably have since I'm not understanding the reasoning given.

[Rubidium]

Sorry, it wasn't a segmentation fault; it was an assertion failure. (I need to pay more attention to what those error windows tell me.) A screen capture of the error window is attached.

There should've been another error message, because this only tells me that there was an error and that the 'errorhandler' was called, but it doesn't tell me the actual error.

[fabca2]

by the way, with last release 9565
I have alway this error : "ini: error in array 'C:\Documents'"
followed by "ini: ignoring invalid NewGRF 'C:\Documents' not found"

note : I have only one GRF : ttrs3w.GRF (v3.01) that I've placed on data folder as usual.
This is occuring when I run ottd. note that I can play a previous game that use TTRS3...

it looks that spaces are a problem now in the path..

[Dextro]

by the way, with last release 9565
I have alway this error : "ini: error in array 'C:\Documents'"
followed by "ini: ignoring invalid NewGRF 'C:\Documents' not found"

note : I have only one GRF : ttrs3w.GRF (v3.01) that I've placed on data folder as usual.
This is occuring when I run ottd. note that I can play a previous game that use TTRS3...

it looks that spaces are a problem now in the path..

Same problem here using Windows Vista 32-bits.

[Red.xiii]

by the way, with last release 9565
I have alway this error : "ini: error in array 'C:\Documents'"
followed by "ini: ignoring invalid NewGRF 'C:\Documents' not found"

note : I have only one GRF : ttrs3w.GRF (v3.01) that I've placed on data folder as usual.
This is occuring when I run ottd. note that I can play a previous game that use TTRS3...

it looks that spaces are a problem now in the path..

Same with latest nightly 9570

[jonty-comp]

Same problem here using Windows Vista 32-bits.

Are all the people that are having this problem on Vista?
'Cause you know, it's all scary with the C:\Users\ in Vista.

[Dextro]

Same problem here using Windows Vista 32-bits.

Are all the people that are having this problem on Vista?
'Cause you know, it's all scary with the C:\Users\ in Vista.

I think vista is to blame yes since openttd is installed in what looks like c:\programas\openttd (portuguese) but is looking for files in C:\Program (what I supose to be c:\program files\openttd)... notice something?

[Red.xiii]

Same problem here using Windows Vista 32-bits.

Are all the people that are having this problem on Vista?
'Cause you know, it's all scary with the C:\Users\ in Vista.

I think vista is to blame yes since openttd is installed in what looks like c:\programas\openttd (portuguese) but is looking for files in C:\Program (what I supose to be c:\program files\openttd)... notice something?

XP Pro here. I agree that its the spaces that kill it as if you place them on the root of C: then it's fine...

[Rubidium]

Sorry for the inconvenience, but:

• I currently have very little time to do anything closely related to leisure (this includes OTTD coding)
• Reverting the patch that 'caused' this doesn't fix the issue that you have trouble with right now; you can 'recreate' this issue in 0.5.0 by just adding a directory with a space in the name in data/ and add a grf to it. Adding that GRF to your list of NewGRFs used by default and closing (thus saving) and then starting OTTD will give you exactly the same problem.
• The fix should be fairly easy; adding some way to escape spaces in 'IniItem->name' when writing to openttd.cfg and unescape them when reading them.

... and that was all the time I could spend freely today

[Red.xiii]

... and that was all the time I could spend freely today

No worries mate, just attempting to help bug fixing. At least there is a work around

[Korenn]

offtopic, but I just wanted to mention:

using

Code: Select all

assert(0)

is very bad coding. It only takes a few seconds to add the actual check that failed before reaching this code in the assertion.

[DaleStan]

And what are you supposed to put as the condition when the actual check is a conglomeration of $LARGENUM different tests?
eg

Code: Select all

if(!strcmp(str, "ONE")) {...}
else if(!strcmp(str, "TWO")) {...}
else if(!strcmp(str, "THREE")) {...}
else if(!strcmp(str, "FOUR")) {...}
else if(!strcmp(str, "FIVE")) {...}
else if(!strcmp(str, "SIX")) {...}
else if(!strcmp(str, "SEVEN")) {...}
else if(!strcmp(str, "EIGHT")) {...}
else if(!strcmp(str, "NINE")) {...}
else if(!strcmp(str, "TEN")) {...}
else assert(0);

It's not exactly a matter of a few seconds to write the correct condition in the assert. And then you've got code duplication problems. If a new condition appears, or (worse yet) if an old condition disappears, then both the if cascade and the assert have to be updated.