Bug ChoiceManager.preChoice() necessity

fredg1

Member
I'm doing a reformatting of ChoiceManager.java, and while doing so, noticed the preChoice() function.

More specifically, I noticed that pretty much everything in this function assumes that you actually were in the corresponding choice adventure when you sent the request.
(for example, it assumes that you were in choice adventure 1000 if it sees the URL choice.php?whichchoice=1000&option=1)

Heck, it doesn't even check if you actually supplied your password hash, only looking at the fact that KoL returned the code 200 ("OK") (which it will always do if you were NOT in a choice adventure to begin with)
responseText is also not being looked at at all at this point.


So, there's definitely "an issue" with preChoice(). My question here is where to go to solve this issue.
  • Is anything in preChoice() really necessary, this early? Would it be possible to move all of this to postChoice0( String, GenericRequest ) instead, or do those really all need to be done before the registering of the adventure by AdventureRequest?
  • If the latter, should the code in this section start looking at responseText?
  • If not possible, should handling be done to make sure you're actually in a choice adventure, before this function?
DISCLAIMER
Again, what I am doing is a REFORMATTING of this file. This means that if you see this post and come up with a solution to this problem, discuss it here instead of taking any initiative, to make sure your work is not wasted by what I'll have done on my side.
 

Veracity

Developer
Staff member
You do not understand the control flow that leads to this method. It does not "assume that you actually were in the corresponding choice adventure when you sent the request". It is called before we register the request, which is done before we actually submit the request.
Which also means that it does not "look at the fact that KoL returned the code 200", since the request has not been submitted yet.
Which also means that it does not look at responseText, since the request has not been submitted yet.

You say there's "definitely "an issue" with preChoice()".

Can you explain to me exactly what "the issue" is, please?
 

fredg1

Member
I may have not understood the control flow leading to this method, but I stand by the fact that it assumes your state (just switching from the past tense to the present tense).

It does things such as modifying properties based on the option field, and registering that a turn was spent.

For example: trying to visit the URL choice.php?whichchoice=142&option=3 will always result in mafia assuming that the war was started, even if you were not in choice adventure 142 (and either way, it wouldn't have worked since you're not even submitting your pwd hash)

Hell, if what you say is true about this being done before the request is even sent, it could be nightly maintenance and these changes would still be done.
 

Veracity

Developer
Staff member
Obviously some of the content of that method should move to a postChoice method of some sort, then.
 

fredg1

Member
that's the thing: it's ALL things like that, which is why I asked if it was vital for this handling to be done there, or if they could be moved to postChoice0
 

fredg1

Member
the only exception that I see so far is for choice 1197, which sets the internal field EatItemRequest.timeSpinnerUsed to true
 

fredg1

Member
(but even then, if you weren't actually using the time spinner, it will never be set back to false, afaik)
 

fredg1

Member
I guess it wouldn't hurt to validate my current understanding of the control flow happening in ChoiceManager/surrounding choice.php requests; can you check if I am correct/how close I am?

My current understanding:
(premise: this is a request being executed, starting with choice.php?)

- ChoiceManager.preChoice() is visited
the URL is looked at;​
ChoiceManager.lastChoice and ChoiceManager.lastDecision are set to the values of the whichchoice and option fields respectively, if both are there.​
Otherwise, they are both set to 0 (meaning we lost any way of knowing what the previous request was either way).​

- ChoiceManager.registerRequest() is visited
Here, we properly "log" that the adventure is about to happen.​
This means things like writing information in the GCLI and session logs, such as​
Code:
[1] The Sleazy Back Alley

- The request is sent to KoL, and the response retrieved

- ChoiceManager.postChoice0() is visited
A few special cases that need to be logged/handled before registering the adventure.​

- we register the encounter with AdventureRequest.registerEncounter(), officially logging that the "encounter" is happening.

- ChoiceManager.postChoice1() is visited
if ChoiceManager.lastChoice is 0 (set in ChoiceManager.preChoice()), we ditch this method for ChoiceManager.visitChoice(), as this is definitely a visit of, instead of a response to, a choice adventure.​
Otherwise, we assume that we successfully went through choice adventure <x>, and process this information accordingly by looking at the response and the url used.​

- we parse the results and show the request in the browser

- ChoiceManager.postChoice2() is visited
Here, we mostly just set some properties that depend on the results of the choice adventure.​


Does this seem accurate?
 

gausie

D̰͕̝͚̤̥̙̐̇̑͗̒e͍͔͎͈͔ͥ̉̔̅́̈l̠̪̜͓̲ͧ̍̈́͛v̻̾ͤe͗̃ͥ̐̊ͬp̔͒ͪ
Staff member
I guess it wouldn't hurt to validate my current understanding of the control flow happening in ChoiceManager/surrounding choice.php requests; can you check if I am correct/how close I am?

My current understanding:
(premise: this is a request being executed, starting with choice.php?)

- ChoiceManager.preChoice() is visited
the URL is looked at;​
ChoiceManager.lastChoice and ChoiceManager.lastDecision are set to the values of the whichchoice and option fields respectively, if both are there.​
Otherwise, they are both set to 0 (meaning we lost any way of knowing what the previous request was either way).​

- ChoiceManager.registerRequest() is visited
Here, we properly "log" that the adventure is about to happen.​
This means things like writing information in the GCLI and session logs, such as​
Code:
[1] The Sleazy Back Alley

- The request is sent to KoL, and the response retrieved

- ChoiceManager.postChoice0() is visited
A few special cases that need to be logged/handled before registering the adventure.​

- we register the encounter with AdventureRequest.registerEncounter(), officially logging that the "encounter" is happening.

- ChoiceManager.postChoice1() is visited
if ChoiceManager.lastChoice is 0 (set in ChoiceManager.preChoice()), we ditch this method for ChoiceManager.visitChoice(), as this is definitely a visit of, instead of a response to, a choice adventure.​
Otherwise, we assume that we successfully went through choice adventure <x>, and process this information accordingly by looking at the response and the url used.​

- we parse the results and show the request in the browser

- ChoiceManager.postChoice2() is visited
Here, we mostly just set some properties that depend on the results of the choice adventure.​


Does this seem accurate?
Once you get to the bottom of this exact summary, it would be great to include a summary as a comment at the top of ChoiceManager (with greater detail next to each function), because I have often wondered about precisely this.
 
Top