retrieveVehicle Has Flawed Logic?

[]V[ȭngoʃïan Σagle ]-[unter 4]

Hey,

So I've been migrating my server files to the new 1.0.4 and reviewing lots of code line by line and have noticed a few things that I think could be an issue for a lot of people going forward.  The biggest thing I'm concerned with at the moment is how the new Virtual Garage system fetches and retrieves vehicles.  I noticed that they added this new column "nickname" to the vehicle table and are using this to identify stored vehicles.  However, this is a bad way to do this because the new nickname field is not unique in the database, and there is nothing in the serverside SQF code to prevent multiple vehicles with the same nickname being stored into the virtual garage.  If someone stores multiple vehicles with the same nickname, then when they go to fetch the vehicle they will always get the first one in the table.

See below what I mean:

exile.ini: storeVehicle

UPDATE vehicle SET territory_id = ?, nickname = ?, last_updated_at = NOW() 

exile.ini: retrieveVehicle

UPDATE vehicle SET territory_id = NULL, last_updated_at = NOW(), nickname = '' WHERE id = ?

exile.ini: confirmVehicleOwnership

SELECT id, territory_id FROM vehicle WHERE nickname = ?

exile_server\code\ExileServer_object_vehicle_network_retrieveVehicleRequest.sqf

	line 33: _vehicleInfo = format["confirmVehicleOwnership:%1", _nickname] call ExileServer_system_database_query_selectSingle;
	line 34: if !((_vehicleInfo select 1) isEqualTo _territoryID) then 
	line 35: {
	line 36:	throw "Vehicle does not belong to this territory!";
	line 37: };
	...
	line 64: format["retrieveVehicle:%1", _vehicleInfo select 0] call ExileServer_system_database_query_fireAndForget;

You can see the confirmVehicleOwnership fetches based on the non-unique 'nickname' column, and only does query_selectSingle so it's only going to get the first row returned, at best, or maybe even break completely if multiples rows are returned, not sure on this exactly.

But I really think someone should re-think this and do this in a different way, because this is guaranteed to cause problems for lots of people going forward.  Also, it's a bit disappointing that the new virtual garage doesnt show what's in the stored vehicles inventories like the ExAd one does.  Showing this somewhere would be nice.

I migrated from the old ExAd Virtual Garage and i'm having some issues fetching vehicles that I know I should be able to fetch, they have the correct territory_id and the database entry looks correct, but when I try to fetch them it says "Vehicle does not belong to this territory!" and I can't figure out why.  I've commented out lines 34-37 in that retrieveVehicleRequest.sqf file for now, just to get around that issue, but I'm still troubleshooting other issues.  Just thought I should post this to hopefully help some others out there who may be struggling.

Just my 2cents.  Feel free to chime in with any ideas on how to fix this, or if I'm wrong and this will never be a problem for anyone please tell me how.

 

UPDATE 2018-03-12:

So I unpacked the exile clientside files and was looking through them and there IS something in the clientside SQF that checks for a unique vehicle "nickname".   See below:

exile_client\code\ExileClient_gui_virtualGarageDialog_event_onConfirmButtonClicked.sqf:

			Line 68: _storedVehicles = _flag getVariable ["ExileTerritoryStoredVehicles", []];
			Line 69: _count = {toLower(_x select 1) isEqualTo toLower(_nickName)} count _storedVehicles;
			Line 70: if (_count > 0) then 
			Line 71: {
			Line 72: 	throw format["'%1' is already in use by another vehicle.", _nickName];
			Line 73: };

Although, I still think it would be better to have this check serverside, or do this a diff way maybe.

Edited March 25, 2018 by ]V[ȭngoʃïan Σagle ]-[unter
Found new info in clientside files.

[]V[ȭngoʃïan Σagle ]-[unter 4]

For those using the id as the nickname in the database, I ran into some errors which I noticed on the clientside log files where the SQF was reading the the nickname as a number instead of as a string, and was causing some issues.

If you migrated from previous ExAd Virtual Garage and are using the id as the nickname, you may run into these issues as well.  I suggest doing the following to fix, use CONCAT(class,id) as the nickname for existing VG entries:

This will set all vehicles stored in the virtual garage to have the nickname be the classname with their row id appended to the end.  This way the name is unique, and it's not read as a number but as a string (This only affects vehicles who have their nickname set as the row id currently):

UPDATE vehicle SET nickname=CONCAT(class,id) WHERE territory_id IS NOT NULL AND nickname=id;

Just thought I'd post this as I came across this a while back and it solved the issues we were having with vehicles using id as nickname.  Hope it helps someone.

[Spazz711 23]

Yeah this could be an ongoing issue.

In the short term you can run an sql script to get it initially working, this will set the nickname to the vehicle ID number.

Spoiler

UPDATE `exile`.`vehicle` SET `nickname`= id WHERE territory_id IS NOT NULL;

 

But I do get what you mean, this could be an issue if people store it with the name 'car'

[]V[ȭngoʃïan Σagle ]-[unter 4]

Hey,

Yeah that is a good temp fix for ppl with existing vehicle garages.  I've also changed the SQL for storeVehicle slightly so that it always uses the id as the nickname, to avoid issues and basically rendering the nickname pointless for now until a better fix comes along.

exile.ini:

-Changed nickname = id

-Changed number of inputs from 3 to 2

[storeVehicle]
SQL1_1 = UPDATE vehicle SET territory_id = ?, nickname = id, last_updated_at = NOW() WHERE id = ?
;;SQL1_1 = UPDATE vehicle SET territory_id = ?, nickname = ?, last_updated_at = NOW() WHERE id = ?
Number of Inputs = 2
SQL1_INPUTS = 1,2

And changed the SQF in the following file a bit:

exile_server\code\ExileServer_object_vehicle_network_storeVehicleRequest.sqf:

	Line 97: //format["storeVehicle:%1:%2:%3", _flagObject getVariable ["ExileDatabaseID", -1], _nickname, _vehicleObject getVariable ["ExileDatabaseID", -1]] call ExileServer_system_database_query_fireAndForget;
	Line 98: format["storeVehicle:%1:%2", _flagObject getVariable ["ExileDatabaseID", -1], _vehicleObject getVariable ["ExileDatabaseID", -1]] call ExileServer_system_database_query_fireAndForget;

 

Edited March 11, 2018 by ]V[ȭngoʃïan Σagle ]-[unter

[ProGEEK 24]

Just stumbled onto this issue myself. This is a good workaround for now.
Why they're referencing an object by a non-unique column is beyond me, They should be using the vehicle ID in all queries etc, that at least is already unique and directly identifies the vehicle..

Edited March 11, 2018 by ProGEEK
Spelling fixes

[Chainsaw Squirrel 144]

Have these been thoroughly tested , hate to do these and it hose things even more .. or unfixable later

 

[geekm0nkey 144]

How about issue recalling a vehicle from the VG where it had be stored previously under 1.0.3? I went through every row, and added a unique number to the nickname field. For some reason every item, construction, territory, vehicle and container had a delete date of today? had to change all of then to null. But none of them show in anyone virtual garage? They are in the database, all have a null for deleted date, all have a unique nick name? new items work.... previous items do not?

[]V[ȭngoʃïan Σagle ]-[unter 4]

1 hour ago, geekm0nkey said:

How about issue recalling a vehicle from the VG where it had be stored previously under 1.0.3? I went through every row, and added a unique number to the nickname field. For some reason every item, construction, territory, vehicle and container had a delete date of today? had to change all of then to null. But none of them show in anyone virtual garage? They are in the database, all have a null for deleted date, all have a unique nick name? new items work.... previous items do not?

Use the query from the previous post to set all vehicles in virtual garages to have the nickname same as their row id, which is unique since it's the primary key.  This is the best option if you have previous virtual garages and want to convert them.

UPDATE `vehicle` SET `nickname`= id WHERE territory_id IS NOT NULL;

I also noticed other issues with the new virtual garage and I'm still troubleshooting through them, I've had to make several changes to the two retrieveVehicle and storeVehicle scripts to get things working better, but it may cause other issues down the road I'm not sure yet.

I saw on another thread that Janski has stopped working on ExAd and the old ExAd apps, including virtual garage, will just not work with the new way they coded the XM8 in 1.0.4.  So, it's up to us to try and convert those ExAd apps into the new format.  I'm going to give it a shot soon as I have time, but still working through this update just to get it playable on our Production server.

Edited March 12, 2018 by ]V[ȭngoʃïan Σagle ]-[unter

[geekm0nkey 144]

28 minutes ago, ]V[ȭngoʃïan Σagle ]-[unter said:

Use the query from the previous post to set all vehicles in virtual garages to have the nickname same as their row id, which is unique since it's the primary key.  This is the best option if you have previous virtual garages and want to convert them.

UPDATE `vehicle` SET `nickname`= id WHERE territory_id IS NOT NULL;

I also noticed other issues with the new virtual garage and I'm still troubleshooting through them, I've had to make several changes to the two retrieveVehicle and storeVehicle scripts to get things working better, but it may cause other issues down the road I'm not sure yet.

I saw on another thread that Janski has stopped working on ExAd and the old ExAd apps, including virtual garage, will just not work with the new way they coded the XM8 in 1.0.4.  So, it's up to us to try and convert those ExAd apps into the new format.  I'm going to give it a shot soon as I have time, but still working through this update just to get it playable on our Production server.

all my entries have a unique 6 digit number in the nickname field..

also, i can attest the @Spazz711 exad workaround works very well.

Edited March 12, 2018 by geekm0nkey
corrected credit.

[Spazz711 23]

ExAd does work, just need to workaround it:

 

Edited March 12, 2018 by Spazz711

[]V[ȭngoʃïan Σagle ]-[unter 4]

Yes, the other parts of ExAd are working for me as well, it's just the ExAd Xm8 Apps that require converting, like I said.

The new format is much easier to create XM8 apps so it shouldn't be too hard.  That thread @Spazz711 posted has good info on how to do it.