Some impressions for new addon directry system. (This is not a bug report nor a complain. I missed old topic about this extension request.)
Pak select window: I personally like previous version's one than this version's one.
if( umgebung_t::default_einstellungen.get_with_private_paks() ) {
// try to read addons from private directory
chdir( umgebung_t::user_dir );
if(!obj_reader_t::load(umgebung_t::objfilename,translator::translate("Loading addon paks ..."))) {
fprintf(stderr, "reading addon object data failed (disabling).\n");
umgebung_t::default_einstellungen.set_with_private_paks( false );
}
I think some people will have trouble with this. Because some people install simutrans in their home folder.
That worked well until now but will have "duplicated" error now.
So, if someone want to install simutrans in theis home folder, they must change folder name.
// not possible for single user
umgebung_t::default_einstellungen.set_with_private_paks( false );
}
sad.
Installing in their home directory should be easy to catch by program_dir==user_dir. But the problem is, that for network games extension are not allowed. Furthermore physically seperating addon and paks would be helpful. (The duplicate object error messages will go soon, to allow also existing object to be overwritten. However, there are lots of places to be changed.)
For the home directory it would be all good if the installed folder is not named simutrans but simutrans.game or so.
I don't know well your network game plan.
Network game requires multiuser install is not a good idea.
Recently, many people use USB device or access from internet cafe, they don't want to use home directly.
Then simutrans must be installed without addons or the checksum (not fully finished yet) will not match and one cannot connect to the server. But imho you are right, simutrans should detect this case.
Tested in r2478.
Duplicated error doesn't happen but still have a problem.
Maybe special freight like p****engers are doubled.
P****engers don't get-off from vehicle or p****engers don't enter stop or etc.
[EDIT]Another problem is...
If player have only one pakset, initial pk selector doesn't appear.
In this case, default value is "with_private_paks ON", thus player can't select "no addon" playing.
And also some game crashed.
http://simutrans-germany.com/files/upload/AI2-01.sve for pak64
1. Install simutrans in home folder
2. start simutrans with addons
3. Load attached savegame
Result:
Within 1 minutes, simutrans crash.
I can reproduce the crash. It happens in the ai, when two ware-description that should match dont match actually (in ai_goods_t line 127, warenr out of bounds).
I could not reproduce this (with my compiled version). But I suspect that the hastable removal does not work as expected. I have added another version for get/remove instead.
And a player with a single pak set need at the momen to edit the simuconf.tab (with_private_paks=0).
I know that.
Then, why multi pakset user have to choice whether they use addon or not every time ? And also single-install player can't use addon select but have to show select button every time ?
It's not good GUI.
[EDIT]Tested in r2481. Unfortunately, problem is not solved yet.
I think the problem is, that duplicate objects are double registered for the xref-magic. And there the first (ie old one, not addon one) wins. Here is a patch, I do not know whether this is correct or not. At least the warning is not triggered in pak64 or pak128 without addons, nor z9999's game behaves weird and crashes.
Index: besch/reader/obj_reader.cc
===================================================================
--- besch/reader/obj_reader.cc (revision 2481)
+++ besch/reader/obj_reader.cc (working copy)
@@ -396,6 +396,10 @@
if(!objtype_loaded->get(name)) {
objtype_loaded->put(name, data);
}
+ else{
+ dbg->warning("obj_reader_t::obj_for_xref","Object %s already registered", name);
+ obj_besch_t *old_data = objtype_loaded->set(name, data);
+ }
}
The laster should win over the current. Otherwise the Xrefs will be worng. Imho I forgot also city cars.
Thanks Dwachs. Your patch solved the problem.
But if all pak should be overwritten by addon, your patch doesn't work like that.
But I don't understand why all pak should be overwritten by addon.
Network game required that ? If so, designing is bad.
And alo, I think default should be "with_private_paks=0". People don't want to read same folder twice.
Can you specify, what exactly does not work?
I tested only the situation, that one directory was loaded twice.
The addon-directory is loaded after the standard pak-directory, so that the addons overwrite existing standard-paks. My patch fixes only some internal mismatchings, e.g. vehicles and factories pointing to standard but ignored good paks.
I'm not prissi, I don't know which is correct behavior.
If I have p****enger.pak with bounus=15 in pak folder, and p****enger.pak with bounus=30 in addon folder, result is bounus=15.
This is not overwritten. I wrote so.
But again, I'm not prissi, I don't know which is correct behavior.
this works for me (rev 2483 with or without my patch). I created an empty addon pak/ folder and copied goods.p****agiere from pak.german. Loading pak alone gives the right (standard pak64) p****engers, loading with addons gives the pak.german p****enger definition.
As I understand the code, prissi's intention is that addon-object override standard objects.
I apologize to you. It worked, I was wrong.
Thank you for your trouble.
:) thanks for testing!
here is a patch with the missing checks for duplicates. The source code compiles at least, but this patch is otherwise untested.
@prissi: please commit my patches here, if you find them correct. I am not sure whether this xref-thing works correctly.
Why was there spam in from of the patch files. Did the forum got hacked? Anyhow, I will look for the patch.
Doing this right I end up with almost the same code for any besch, i.e. adding it to a stringhashtable. Imho this should be done by the object itself (or it is rather done anyway by the xref reader, if I see this correctly. Thus laden_abschliessen() would just need a pointer to the stringhashtable and could work its magic there then. Unforutunately this is probably to much C++ magic for me to be done properly :(