SVN [1267] - Alt Update Bug

Posts from previous Beta sessions

SVN [1267] - Alt Update Bug

Postby Feithar » Sun Aug 26, 2007 2:30 am

Here's the setup:

We use guild officer notes to keep track of to whom each of our alt's belongs. This works fine, except that we have a separate guild rank for all of our alts, and this separate rank does not have access to see officer notes.

The problem comes when we upload our character/guild data. As an example, if I am playing my main, GuildProfiler picks up the officer notes, then when my data is automatically uploaded when I log out, everything works fine. However, when I play one of my alts, GuildProfiler can not see the officer notes. When I log out now and upload my data, all the alt information gets wipes out of the roster.

The solution is probably to modify the member list module so that if the match field is officer notes, and the CharacterProfiler.lua file has the ["HasOfficerNote"] = false line in the ScanInfo section, that alt data should not be updated. I'll take a look and see if I can hack something together, but I just wanted to post this here in case there's a fix I'm overlooking.

Another alternative is to just have an option so if a bunch of characters get uploaded in the same CharacterProfiler.lua file, then they're all considered to be alts of one of the characters. That way we wouldn't have to mess with parsing notes at all. I had this working in roster 1.7.3, and could probably adapt the source code for the new version if anyone else is interested.
User avatar
Feithar
WR.net Apprentice
WR.net Apprentice
 
Posts: 15
Joined: Sun Nov 26, 2006 7:58 am

Re: SVN [1267] - Alt Update Bug

Postby Feithar » Sun Aug 26, 2007 4:23 am

Here's my fix for the above issue.

Interestingly enough, when I try to store data in the memberslistUpdate object, they seem to get reset between each function call. Is something in the base code recreating the object continuously? If so, where is the proper place to store data that needs to be kept between calls to guild_pre and guild?

The following patch should be applied to the update_hook.php file. BTW, I'd like to add that at first glance, I really like the way addons are handled in the new version. Should be fun to play with if I can ever find some free time... :)

Code: Select all
20a21,26
> // Keeps track of if officer notes are present in the guild profiler data.
> // I'd keep this in the memberslistUpdate class, but it keeps getting reset with
> // each function call. Is the object being recreated with each call? If so, where
> // do addon designers store data that needs to be tracked between calls?
> $hasOfficerNote = false;
>    
68a75,111
>     * Keep track of whether or not officer notes are available in the profile data.
>     *
>     * @param array $guild
>     *      CP.lua guild data
>     */
>    function guild_pre ( $guild )
>    {
>       global $hasOfficerNote;
>
>       $hasOfficerNote = false;
>       
>       // --[ Check if this update type is enables ]--
>       if(( $this->data['config']['update_type'] & 1 ) == 0 )
>       {
>          // prevent the addon name from being displayed
>          $this->messages = '';
>          return true;
>       }
>
>       if( isset($guild['ScanInfo']) && is_array($guild['ScanInfo']) )
>       {
>          $scanInfo = $guild['ScanInfo'];
>          if ( isset($scanInfo['HasOfficerNote']) )
>          {
>             $hasOfficerNote = $scanInfo['HasOfficerNote'];
>          }
>       }
>       
>       if ( $hasOfficerNote )
>       {
>          $this->messages .= ' - Officer notes present in guild profile.';
>       } else {
>          $this->messages .= ' - Officer notes NOT present in guild profile.';
>       }
>    }
>    
>    /**
78a122
>       global $hasOfficerNote;
87a132,141
>       // --[ Check to see if we're scanning officer notes. If we are, make sure they're present in the profile data ]--
>       if ( $this->data['config']['getmain_field'] == 'OfficerNote' )
>       {
>          if ( !$hasOfficerNote )
>          {
>             $this->messages .= ' - Unable to read officer notes - aborting alt scan.';
>             return true;
>          }
>       }
>       
User avatar
Feithar
WR.net Apprentice
WR.net Apprentice
 
Posts: 15
Joined: Sun Nov 26, 2006 7:58 am

SVN [1267] - Alt Update Bug

Postby PleegWat » Sun Aug 26, 2007 6:09 am

Thanks for the headsup. Back in AltMonitor, the note data was pulled from DB. The updater doesn't insert notes if they're not in the data, so I always had valid notes.
Now, in 1.8^h^h^h2.0, we're working straight on the lua data, so if there's no note, it needs to detect that (like you noticed)

I'll take a look in and fix it. I'll also try to verify why the data in the update object isn't persisting for you. It should persist, and the code for per-upload alt matching relies on the data persisting. If I'm remembering correctly.
I <3 /bin/bash
User avatar
PleegWat
WoWRoster.net Dev Team
WoWRoster.net Dev Team
 
Posts: 1636
Joined: Tue Jul 04, 2006 1:43 pm

SVN [1267] - Alt Update Bug

Postby zanix » Sun Aug 26, 2007 9:11 am

This check probably should be added in the main update code as well
Read the Forum Rules, the WiKi, and Search before posting!
WoWRoster v2.1 - SigGen v0.3.3.523 - WoWRosterDF
User avatar
zanix
Admin
Admin
WoWRoster.net Dev Team
WoWRoster.net Dev Team
UA/UU Developer
UA/UU Developer
 
Posts: 5546
Joined: Mon Jul 03, 2006 8:29 am
Location: Idaho Falls, Idaho
Realm: Doomhammer (PvE) - US

Re: SVN [1267] - Alt Update Bug

Postby Feithar » Sun Aug 26, 2007 9:43 am

Might have found the answer to my non-persistent data.

Apparently, when you use foreach in PHP, it creates a copy of the array, rather than iterating through the original. The effect is that changes made to the array within the loop will not be made to the original objects, but rather to the copies.

Here's a link to a thread that seems to explain it:

http://groups.google.com/group/comp.lang.php/msg/bf35784c733ef439

It says that there's a feature in PHP 5 that lets you work around it, but unfortunately my web provider only supports PHP 4.4. Hopefully you won't consider this a good enough reason to move to PHP 5... :wink:

Hope this helps. I'll do some playing on my end, and see if I can get something working. In the mean time, the work around with using a global seems to be working fine.

Also, I think you're missing a global $roster; at the beginning of the addon_hook function in update.lib.php. I was getting PHP errors when I was doing my updates. Unfortunately, I didn't write down the error messages, but I think it was along the lines of $roster not being defined in lines 252 and 253.
User avatar
Feithar
WR.net Apprentice
WR.net Apprentice
 
Posts: 15
Joined: Sun Nov 26, 2006 7:58 am

SVN [1267] - Alt Update Bug

Postby zanix » Sun Aug 26, 2007 10:08 am

Feithar wrote:Also, I think you're missing a global $roster; at the beginning of the addon_hook function in update.lib.php. I was getting PHP errors when I was doing my updates. Unfortunately, I didn't write down the error messages, but I think it was along the lines of $roster not being defined in lines 252 and 253.
You are right
This is now fixed in the svn as well as update_guild_members now checks if the uploader 'hasofficernote'
Last edited by zanix on Sun Aug 26, 2007 10:12 am, edited 2 times in total.
Read the Forum Rules, the WiKi, and Search before posting!
WoWRoster v2.1 - SigGen v0.3.3.523 - WoWRosterDF
User avatar
zanix
Admin
Admin
WoWRoster.net Dev Team
WoWRoster.net Dev Team
UA/UU Developer
UA/UU Developer
 
Posts: 5546
Joined: Mon Jul 03, 2006 8:29 am
Location: Idaho Falls, Idaho
Realm: Doomhammer (PvE) - US

SVN [1267] - Alt Update Bug

Postby PleegWat » Sun Aug 26, 2007 7:17 pm

I think I knew about the foreach construct being by copy, but I don't see how it's relevant here?

Code: Select all
foreach( $array as $key => $value )
{
// This will change the value within the loop only
$value = 'x'

// This will change the value in the array
$array[$key] = 'x'
}

foreach( $array as $key => &$value )
{
// Here both methods will change the value in the main array, and the change will persist
$value = 'x'
$array[$key] = 'x'
}


All IIRC. The second loop only works in php5.
I <3 /bin/bash
User avatar
PleegWat
WoWRoster.net Dev Team
WoWRoster.net Dev Team
 
Posts: 1636
Joined: Tue Jul 04, 2006 1:43 pm

Re: SVN [1267] - Alt Update Bug

Postby Feithar » Sun Aug 26, 2007 8:10 pm

Here's a quick example, which shows what I was trying to do.

First each of the addon's update hook objects are constructed. By default, let's say we set the hasOfficerNotes class member to be equal to 'unset'

Code: Select all
// Construction code here.
// For each addon, $addon->hadOfficerNotes = 'unset'


Later on, we do our loop to call the guild_pre method for each addon:

Code: Select all
foreach($addons as $addon) {
  $addon->hasOfficerNote = $scanInfo['HasOfficerNote'];
}


At this point, we would expect the hasOfficerNote member for each addon to be equal to either true of false, depending on the scan info.

Now, we try calling the guild method for each addon. In this example, we'll just print the value...

Code: Select all
foreach($addons as $addon) {
  echo $addon->hasOfficerNote;
}


Unfortunately, this will always print 'unset', which was the value we assigned in the constructor, because we never modified the original object in the previous foreach loop - just the copy

Hope that helps. The reason I had to do it this way is because guild information is passed to guild_pre, but only character information is passed to guild, so while you can read the scan info in guild_pre, that data is not available in the guild method.

I'll try the new build that was posted last night later today. It sounds like zanix made some changes to take into account missing officer notes elsewhere in the project, so I'll post here and let you know if that did the trick.

Thanks! :D
User avatar
Feithar
WR.net Apprentice
WR.net Apprentice
 
Posts: 15
Joined: Sun Nov 26, 2006 7:58 am

Re: SVN [1267] - Alt Update Bug

Postby Feithar » Sun Aug 26, 2007 9:09 pm

Still broken in 1282. :(

Also, I tried (on the 1267 build) replacing in the addon_hook function of the update.lib.php file:

Code: Select all
foreach( $this->addons as $addon )


with

Code: Select all
while(list(, $addon) = each($this->addons))


Now, I'm definitely not a PHP guru, so maybe I got the syntax wrong, but that change definitely broke everything, but the idea was that the while look would iterate over the original objects instead of copies, which would allow us to keep track of data between calls to guild_pre, and guild.

Hope that helps.
User avatar
Feithar
WR.net Apprentice
WR.net Apprentice
 
Posts: 15
Joined: Sun Nov 26, 2006 7:58 am

SVN [1267] - Alt Update Bug

Postby zanix » Mon Aug 27, 2007 1:37 am

I think we just need it left up to the addon itself to detect and set this rather than in update.lib.php

PleegWat has made a change to memberslist in the svn that solves this problem
Read the Forum Rules, the WiKi, and Search before posting!
WoWRoster v2.1 - SigGen v0.3.3.523 - WoWRosterDF
User avatar
zanix
Admin
Admin
WoWRoster.net Dev Team
WoWRoster.net Dev Team
UA/UU Developer
UA/UU Developer
 
Posts: 5546
Joined: Mon Jul 03, 2006 8:29 am
Location: Idaho Falls, Idaho
Realm: Doomhammer (PvE) - US

SVN [1267] - Alt Update Bug

Postby PleegWat » Mon Aug 27, 2007 5:35 am

I've hopefully fixed this within the memberslist addon. The foreach by-copy limitation will definitely hit you if you implement it like that.

The best alternative to this approach would be passing the whole guild object to guild(_member) triggers, either instead of or in addition to the member object.
I <3 /bin/bash
User avatar
PleegWat
WoWRoster.net Dev Team
WoWRoster.net Dev Team
 
Posts: 1636
Joined: Tue Jul 04, 2006 1:43 pm


Return to Archived

Who is online

Users browsing this forum: No registered users and 0 guests

cron