SQL error in task system

[Darkscis]

Wondering if anyone can assist.

I am getting an SQL syntax error when using quest::enabletask() and quest::disabletask() and I can't seem to work out why. Code is from approx March this year.

quest::disabletask() error states:
[MySQL Error] 1064: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '012)TE FROM character_enabledtasks WHERE charid = 659774 AND (' at line 1 --- 012(TE FROM character_enabledtasks WHERE charid = 659774 AND (

quest::enabletask() error states:
[MySQL Error] 1064: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '659774, 302)character_enabledtasks (charid, taskid) VALUES' at line 1 --- (659774, 302)character_enabledtasks (charid, taskid) VALUES

After the script runs quest::enable task I check the db and nothing has been added to the character_enabledtasks table - the task selector window DOES show the quest though which is weird - I was under the impression it should not if there was no entry. Could be related I guess.

[Uleat]

There could be one or more issues at work here.

I'm pushing a fix for both case scenarios:

- No task values due to an empty task list (creating an invalid query fragment)

- Order of precedence issue regarding an unscoped, inline conditional check next to an overloaded bitwise operator ('<<' is processed before '?')

queryStream << i ? StringFormat("taskid = %i ", tasksDisabled[i]): StringFormat("OR taskid = %i ", tasksDisabled[i]);

You can either pull in the latest repo and update your database..

..or click on the link below and apply the changes there manually to your server code - if you don't want to update your code.

commit link: https://github.com/EQEmu/Server/comm...a2c5782a34f38d

[Darkscis]

Thanks Uleat!

Appreciate the quick reply. I will apply the fix and recompile today then let you know.

[Darkscis]

Didn't work. After turning on logging I can see that it is adding the charid and taskid to the beginning of the string.

i.e. it is trying to run;
(659774,306)character_enabledtasks (charid,taskid) VALUES

instead of REPLACE INTO character_enabledtasks ...

Same thing for quest::disabletask.

EDIT: Fixed by changing both entries to this (2nd Edit: Also I had to remove your brackets for the order of precedence around the disabletask conditional as it caused issues);

std::stringstream queryStream;
queryStream << "REPLACE INTO character_enabledtasks (charid, taskid) VALUES ";

std::stringstream queryStream;
queryStream << StringFormat("DELETE FROM character_enabledtasks WHERE charid = %i AND (", charID);

Thanks again for your help!!

[Uleat]

Nice find!

http://stackoverflow.com/questions/3...ng-constructor

Not sure who wrote that originally..but, yes, the stream is initialized with the provided string without setting the stream's write position - it is still set to '0'


Personally, I don't use the string constructor of a stringstream, so I definitely missed that one.

I'll push a fix for that today.


EDIT:

EDIT: Fixed by changing both entries to this (2nd Edit: Also I had to remove your brackets for the order of precedence around the disabletask conditional as it caused issues);

Can you elaborate on the issue caused by this?

There is a scope declaration around the enabletask query builder that does the exact same thing.


EDIT2: I pushed with the inner scope declaration. If you can explain why this is an issue, I'll look at it and either fix or address the issue.

[Darkscis]

Sorry quick update - the error was not your order of precedence brackets, but the conditional being the wrong way around. I had to reverse the options.

(sorry - dont have exact lines as I don't know how to work git :P )
Around line ~775

Original:
queryStream << i ? StringFormat("taskid = %i ", tasksDisabled[i]): StringFormat("OR taskid = %i ", tasksDisabled[i]);

This resulted in a query that looked like this with only a single task passed to it;
DELETE FROM character_enabledtasks WHERE charid = %i AND (OR taskid = %i " because of the conditional.

New:
queryStream << i ? StringFormat("OR taskid = %i ", tasksDisabled[i]) : StringFormat("taskid = %i ", tasksDisabled[i]);

This now results in the correct SQL statement;
DELETE FROM character_enabledtasks WHERE charid = %i AND (taskid = %i
or;
DELETE FROM character_enabledtasks WHERE charid = %i AND (taskid = %i OR taskid = %i - etc for multiple values.

So both these queries now work correctly and they replace and delete from the correct table. They still don't seem to function quite right though - I think it might be the enabled tasks list itself but I really don't have the skills to delve deeply into that.

Basically what happens is;
I enable 3 tasks, 302, 305 and 306 individually after doing a qglobal check for a daily qglobal.

if ($client->GetGlobal("daily_302") != 1 && $faction <= 7) { quest::enabletask(302); }
	if ($client->GetGlobal("daily_305") != 1 && $faction <= 7) { quest::enabletask(305); }
	if ($client->GetGlobal("daily_306") != 1 && $faction <= 7) { quest::enabletask(306); }

This results in correctly entering the entries into the character_enabledtasks db and then the task selector displays the 3 quests - so far so good. Task debugging shows the enabled task list has those 3 in it. I handled quest completion via perl as well, so I stay in zone and complete the task which then disables the task and sets the daily global as below.

if ($task_id == 302)
	{
		##Give rewards for completing task: Clear the Flooded Caves (Daily)
		$client->AddAAPoints(5); ## 5 AA points
		$client->AddMoneyToPP(0, 0, 0, 3333, 1); ## 3,333 platinum
		quest::summonitem(quest::ChooseRandom(@rewards), 1); ## Give a random reward from @rewards
		quest::summonitem(200175); ## Award charm increase token
		quest::disabletask(302); ## Disable the task and set the daily global
		$client->SetGlobal("daily_302", 1, 5, "H23"); ## 23 hours so people can do it basically at the same time each day
		$client->Message(15, "You have been awarded 5 AA points, 3,333 platinum, a charm token and a random reward for completing this task.");
	}

So all of that works fine, they get their rewards and the global is set. quest::disable tasks runs but here is where it gets weird. The logging shows that the task system removes 302 from the enabled task list correctly and tells me only 305 and 306 are enabled - the mysql logging shows me that the following statement runs;

DELETE FROM character_enabledtasks WHERE charid = 659774 AND (taskid = (0))

Now presumably that is the index of the task because if I do quest::disabletask(302, 305, 306) after zoning it shows taskid - (012)

The thing is - most of the time it says it runs the statement but 0 rows were affected and the 659774, 302 values are not removed from the enabledtasks table. I'm not sure if I can somehow see what is in index 0 but it seems like it isn't generating it quite right. After zoning however, if I write an EVENT_SAY script just to disable all 3 it will work perfect.

Not sure if that makes much sense, it's difficult to explain but I am happy to hook up some time and show you.

EDIT: Could it be because I am not using sequential task id's?

[Uleat]

Check out this posting: http://stackoverflow.com/questions/5...-if-get-parsed

Without that scope enclosure for the inline conditional, this is what results to the query building.


You can verify what is going on by turning on your MySQL logging before triggering the enable/disable tasks.


EDIT:

[enable]
#logs set gmsay 36 3

[disable]
#logs set gmsay 36 0

[Darkscis]

Sorry, didn't get a chance to take another look over the weekend. I will check this out tonight (AU time) and try to see what actual query is being sent and why it affects 0 rows.

[Darkscis]

Update: it all appears to be working now after the changes I posted above (reversing the disabletask conditional). For the quest globals thing I just disabled all quests in the taskset right above my if GetGlobal checks to enablethem. Works perfectly now, successfully managing 10 different dailies using task sets.