Revils

another problem related to fishing, from a well know wiki:

As of 3.1, there is no longer a minimum skill requirement to fish in any zone, and all casts will catch something and award a chance for a skill-up. However, if your skill level is too low for a zone, you will catch mostly vendor trash items. The skill level required to guarantee "no junk" catches is equivalent to the old "no get away" level.

while in actual version we still got:

DEBUG_LOG("Fishing check (skill: %i zone min skill: %i chance %i roll: %i",skill,zone_skill,chance,roll);

if (skill >= zone_skill && chance >= roll)

{

.

.

//fish catched

player->UpdateFishingSkill();

Using mirror image while in raid will make everyone appear with the caster name, I think there's something wrong on SMSG_MIRRORIMAGE_DATA implementation on WorldSession::HandleMirrorImageDataRequest

nice catch

it works fine to me

uint32 count = pQuest->GetSrcItemCount();

if( count <= 0 )

if count is unsigned how it can be less than 0?

anyway I confirm that often questitems are no more removed after quest complete

edit:

same odd check present in Player::GiveQuestSourceItem, I think it's useless anyway since there's check already on load from DB

my apologize, I hope it's fine now, I didn't notice that git-gui do not include all required informations, I will use command line now

What bug does the patch fix? What features does the patch add?

fixed a bug/typo in SelectMountByAreaAndSkill that prevent to summon a special flying mount.

For which repository revision was the patch created?

9992

Is there a thread in the bug report section or at lighthouse? If yes, please add a link to the thread.

none

Who has been writing this patch? Please include either forum user names or email addresses.

me

diff --git a/src/game/Spell.cpp b/src/game/Spell.cpp
index 0c1d69b..508c147 100644
--- a/src/game/Spell.cpp
+++ b/src/game/Spell.cpp
@@ -6614,8 +6614,10 @@ void Spell::SelectMountByAreaAndSkill(Unit* target, uint32 spellId75, uint32 spe

                            // speed higher than 280 replace it
                            if (mountSpeed > 280)
+                            {
                                target->CastSpell(target, spellIdSpecial, true);
-                            return;
+                                return;
+                            }
                        }
                    }
                }

simply: if we summon a special flying mount with spellIdSpecial > 0 and we "know" another mount with mountSpeed <= 280 the function will "return" without casting a spell

I was working to this achievement too and, as far I recall, that's what I've found:

on every kill, the timer is extended (=reset) on AchievementMgr::SetCriteriaProgress

    if(entry->timeLimit)
   {
       time_t now = time(NULL);
       if(time_t(progress->date + entry->timeLimit) < now)
           progress->counter = 1;

       // also it seems illogical, the timeframe will be extended at every criteria update
       progress->date = now;
   }

and on struct AchievementCriteriaEntry there's

    //uint32 unk1;                                          // 28 Alway appears with timed events
                                                           // for timed spells it is spell id for
                                                           // timed kills it is creature id
   uint32  timeLimit;                                      // 29 time limit in seconds

so maybe the previus part of the code should check unk1 flag to not extend the timer in this case

This will also fix main tank and main assistant flags: http://paste2.org/p/832080

I know it's a really ugly patch, it was just for testing the opcodes and see what's wrong in the code.

IMHO assistant,main assistant and main tank should be handle as a generic member flag rather than keeping separated and stored differently on DB so I think this part of the code about raid group need more work

it's ok , we all are here to learn and collaborate

If you look at the first post by Laise, you can see i just recycled that part

and yes, it compiles

@ Janu, as far as i can see...exactly the same code

that 'if' WON'T compile and no, that's not Laise code

you can't declare a variable just after a 'if' and if you put it inside the bracktes, it will only "lives" inside that scope

did you even try to compile this? :confused:

+ if (m_spellInfo->Id == 72286)

+ uint32 spellid = skillval >= 300 ? 72284 : 72283;

+ else if (m_spellInfo->Id == 75614)

+ uint32 spellid = skillval >= 300 ? 76153 : 75617;

maybe:

+ uint32 spellid = 0;

+ if (m_spellInfo->Id == 72286)

+ spellid = skillval >= 300 ? 72284 : 72283;

+ else if (m_spellInfo->Id == 75614)

+ spellid = skillval >= 300 ? 76153 : 75617;

I made a patch for this problem:

Thank you very much, it works!

good catch, really :eek:

EDIT2:

nevermind

Confirmed with 9839

I did some test on debug and I didn't find any problem on code so, like darkstalker wrote, I suspect that there's something wrong on SMSG_GROUP_LIST implementation in Group::SendUpdate().

I noticed that this part was commited by tomrus88 on 2009-11-18 01:50:19 while 33x was still in PTR, so maybe the message changed when this version went live?

i've got this issue too. haven't checked logs so far (havn't time) found http://code.google.com/p/trinitycore/source/detail?r=50ede8e9dc8a83776fb6a0bfe17af9391c17b625# nice comment in commit message, maybe helpful for someone, cause implementation seems hacky

Maybe I'm missing something but this was already done time ago... http://github.com/mangos/mangos/commit/d6eccbd8e3ad269a129789c11c7e97d25549fcc7

In actual version, mangos 3.x, the problem is due that dispell is considered a positive spell, so it "goes through" the bubble, while mass dispell is negative so it's blocked by DS while it's should be the opposite.

An hackish,bad way, could be to marking directly dispell as negative and mass dispell as positive spells in this way

bool IsPositiveEffect(uint32 spellId, SpellEffectIndex effIndex)

{

SpellEntry const *spellproto = sSpellStore.LookupEntry(spellId);

if (!spellproto) return false;

+ if (spellId == 32375 || spellId == 32592) // mass dispell

+ return true;

+ if (spellId == 527 || spellId == 988) // dispell magic

+ return false;

After this you'll got the problem that mass dispell won't prioritize removing of enemy immunity (DS or Iceblock) versus normal buff but this requires another hackish patch :eek:

the "player->isAlive()" seems to be the problem here, maybe you can keep the "player->GetMapId() != GetId()" check.

I think that "player->isAlive()" is to still allows entering players in ghost mode after they release from corpse in instance while "player->GetMapId() != GetId()" should allow players to be summoned while they are already inside the instance (eg. by Warlock summons) but I don't know if those are really "offy" behaviour

why not something like this?

@@ -6022,10 +6022,22 @@ void Aura::HandleSpellSpecificBoosts(bool apply)
                    }
                }
                else
                    return;
            }
+            else if (m_spellProto->SpellFamilyFlags & UI64LIT(0x0000000000000800)) // Arcane Missile
+            {
+                if (!apply && (m_removeMode == AURA_REMOVE_BY_DEFAULT))
+                {            
+                    if (Unit* caster = GetCaster())
+                        caster->RemoveAurasDueToSpell(36032); // Arcane Blast debuff
+                    else
+                        return;
+                }
+                else
+                    return;
+            }

            switch(GetId())
            {
                case 11129:                                 // Combustion (remove triggered aura stack)
                {

sorry to bump this but the actual implementation in master is wrong

I can confirm the bug, I'm going to try your patch

Well I kind of wrote code that just checks if we're still at the same mail by comparing the ID with the last ID

you're assuming that the DB will return the resulset already ordered while it depends by how the index on the fileds are defined and I'm not positively sure that mysql on a query join always orders the result using the primary keys.

On other hand adding a sort, on a large resulset, will impact on performance negatively.

I can confrim too, often the spirt doesn't automatically cast http://www.wowhead.com/?spell=2584

I know but, like it was done on other patches, maybe teleport to the actual position it's better idea since with SPELL_FAILED_TRY_AGAIN we lose too often the remove stuns and roots effect of blink.

for what I know, blink never fails so I don't understand why there are still those checks in CheckCast() with return SPELL_FAILED_TRY_AGAIN.