Skip to main content ↑ ↓
Topic: r2476- Addon directry (Read 6072 times) previous topic - next topic

r2476- Addon directry

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.

Code: [Select]
	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.

Code: [Select]
		// not possible for single user
		umgebung_t::default_einstellungen.set_with_private_paks( false );
	}

sad.

Re: r2476- Addon directry

Reply #1 –
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.

Re: r2476- Addon directry

Reply #2 –
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.

Re: r2476- Addon directry

Reply #3 –
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.

Re: r2476- Addon directry

Reply #4 –
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.

Quote
FATAL ERROR: vector_tpl<T>::[]
index out of bounds: 1 not in 0..0
Aborting program execution ...

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.


Quote
sim.exe caused an Access Violation at location 005b203c in module sim.exe Reading from location 00000070.

Registers:
eax=00000070 ebx=0064bf68 ecx=00000000 edx=00000000 esi=01c9d77f edi=0744b3f2
eip=005b203c esp=0023e9a0 ebp=0023fa18 iopl=0         nv up ei pl nz na pe nc
cs=001b  ss=0023  ds=0023  es=0023  fs=003b  gs=0000             efl=00000202

Call stack:
005B203C  sim.exe:005B203C  display_calc_proportional_string_len_width(char const*, unsigned)  simgraph16.cc:2510
005B60A1  sim.exe:005B60A1  WinMain  simsys_w16.cc:774
00401247  sim.exe:00401247
00401298  sim.exe:00401298
7C817067  kernel32.dll:7C817067  RegisterWaitForInputIdle

Re: r2476- Addon directry

Reply #5 –
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).
Parsley, sage, rosemary, and maggikraut.

Re: r2476- Addon directry

Reply #6 –
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).

Re: r2476- Addon directry

Reply #7 –
Quote from: prissi – on
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.

Re: r2476- Addon directry

Reply #8 –
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.

Code: [Select]
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);
+       }
 }


Parsley, sage, rosemary, and maggikraut.

Re: r2476- Addon directry

Reply #9 –
The laster should win over the current. Otherwise the Xrefs will be worng. Imho I forgot also city cars.

Re: r2476- Addon directry

Reply #10 –
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.

Re: r2476- Addon directry

Reply #11 –
Quote from: z9999 – on
Thanks Dwachs. Your patch solved the problem.
But if all pak should be overwritten by addon, your patch doesn't work like that.
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.
Parsley, sage, rosemary, and maggikraut.

Re: r2476- Addon directry

Reply #12 –
Quote from: Dwachs – on
Can you specify, what exactly does not work?

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.

Re: r2476- Addon directry

Reply #13 –
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.
Parsley, sage, rosemary, and maggikraut.

Re: r2476- Addon directry

Reply #14 –
I apologize to you. It worked, I was wrong.
Thank you for your trouble.

Re: r2476- Addon directry

Reply #15 –
 :) thanks for testing!
Parsley, sage, rosemary, and maggikraut.

Re: r2476- Addon directry

Reply #16 –
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.
Parsley, sage, rosemary, and maggikraut.
 

Re: r2476- Addon directry

Reply #17 –
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 :(