Dupe Fixes

~~WurstbrotQT~~ · 01/18/2014, 20:49

I don't know whether this is already released or not, but here are some fixes concerning the guildbank.
For both fixes you have to edit the DPSrvr.cpp.

Fix 1 (dupe via put items to guildbank):

Look for:

Code:

CDPSrvr::OnPutItemGuildBank( CAr & ar, DPID dpidCache, DPID dpidUser, LPBYTE lpBuf, u_long uBufSize )

Replace:

Code:

		if( (int)( nItemNum ) > pItemElem->m_nItemNum )
			nItemNum = pItemElem->m_nItemNum;
		if( nItemNum < 1 )
			nItemNum = 1;

with

Code:

		if( (short)( nItemNum ) > pItemElem->m_nItemNum )
			nItemNum = pItemElem->m_nItemNum;
		if( (short)(nItemNum) < 1 )
			nItemNum = 1;

Fix 2 (dupe via get item from guildbank):

Look for:

Code:

CDPSrvr::OnGetItemGuildBank( CAr & ar, DPID dpidCache, DPID dpidUser, LPBYTE lpBuf, u_long uBufSize )

Replace:

Code:

				if( (int)( dwItemNum ) > pItemElem->m_nItemNum )
					dwItemNum = pItemElem->m_nItemNum;
				if( dwItemNum < 1 )
					dwItemNum	= 1;

with

Code:

				if( (short)( dwItemNum ) > pItemElem->m_nItemNum )
					dwItemNum = pItemElem->m_nItemNum;
				if( (short)(dwItemNum) < 1 )
					dwItemNum	= 1;

That's it.

Best regards.

TrøublêMakêr · 01/19/2014, 02:07

Bin mir mit Short nicht sicher. Bei mir war es so, dass man nur 32k aus Bank nur entnehmen konnte. Man kann es auch anders lösen.

lanzelord996 · 01/19/2014, 10:55

Short kannn einen Wertebereich zwischen -32767 und 32767 annehmen.

Мentus · 01/19/2014, 12:06

Ganzzahlüberlauf ? Wikipedia

..

Und hier ist es um einiges sicherer gelöst und seit Ewigkeiten released.

~~WurstbrotQT~~ · 01/19/2014, 12:34

Quote:

Originally Posted by Мentus

Ganzzahlüberlauf ? Wikipedia

..

Und hier ist es um einiges sicherer gelöst und seit Ewigkeiten released.

Ich wüsste nicht warum es sicherer wäre, zuerst den 32-Bit Integer zu prüfen und dann in einen 16-Bit Integer zu casten o.O

Мentus · 01/19/2014, 12:56

Quote:

Originally Posted by WurstbrotQT

Ich wüsste nicht warum es sicherer wäre, zuerst den 32-Bit Integer zu prüfen und dann in einen 16-Bit Integer zu casten o.O

Wieso überhaupt casten? Verstehe den Sinn dahinter nicht. Ich weis nicht wo du deine Programmierkünste aufgefasst hast, aber in meinen Büchern wird solch ein Cast moglichst vermieden. Außer er ist unumgänglich.

Wieso sollte man überhaupt vom Client ein "DWORD" senden lassen? Wenn das Maximum eh bei 9999 liegt? Erklär mal.

~~WurstbrotQT~~ · 01/19/2014, 13:31

Quote:

Originally Posted by Мentus

Wieso überhaupt casten? Verstehe den Sinn dahinter nicht. Ich weis nicht wo du deine Programmierkünste aufgefasst hast, aber in meinen Büchern wird solch ein Cast moglichst vermieden. Außer er ist unumgänglich.

Wieso sollte man überhaupt vom Client ein "DWORD" senden lassen? Wenn das Maximum eh bei 9999 liegt? Erklär mal.

Wenn du danach gehst, müsstest du deinen ganzen Source umschreiben.

Mir persönlich ist es wayne, ob ich 2 oder 4 Byte sende und diese dann caste, auch wenn deine Bücher dagegen sprechen. Deine Bücher sprechen wahrscheinlich gegen jede 2. Zeile im Flyff Sourcecode, kannst ihn ja mal Zeile für Zeile durchgehen und überprüfen, ob was in deinen Büchern dagegen spricht.

Am sinnvollsten wäre es, den Client auch ein short schicken zu lassen, aber dann müsstest du auch wieder casten, wo dein Büchlein ja vehement gegen ist, also musst du quasi sämtliche Item Strukturen im Source umschreiben, damit dein Büchlein nicht mehr meckert.

Der Cast macht Sinn, weil, wenn du mal einen genaueren Blick auf die Zeile wirfst

Code:

itemElem.m_nItemNum	= (short)nItemNum;

Und auf jene:

Code:

				pUser->RemoveItem( (BYTE)( nId ), (short)( nItemNum ) );
				UpdateGuildBank( pGuild, GUILD_PUT_ITEM, 0, pUser->m_idPlayer, &itemElem, 0, (short)( nItemNum ) );

und dir nochmal den Link, den du voller Stolz gepostet hast, anguckst, siehst auch du, dass ein 32-Bit Integer ebenfalls zu einem Overflow führen kann. Wie schade, dass das ganze unsigniert ist und wie schade, dass uns am Ende einfach mal 16 Bit fehlen.

Мentus · 01/19/2014, 14:25

Quote:
Originally Posted by WurstbrotQT
Wenn du danach gehst, müsstest du deinen ganzen Source umschreiben.

Mir persönlich ist es wayne, ob ich 2 oder 4 Byte sende und diese dann caste, auch wenn deine Bücher dagegen sprechen. Deine Bücher sprechen wahrscheinlich gegen jede 2. Zeile im Flyff Sourcecode, kannst ihn ja mal Zeile für Zeile durchgehen und überprüfen, ob was in deinen Büchern dagegen spricht.

Am sinnvollsten wäre es, den Client auch ein short schicken zu lassen, aber dann müsstest du auch wieder casten, wo dein Büchlein ja vehement gegen ist, also musst du quasi sämtliche Item Strukturen im Source umschreiben, damit dein Büchlein nicht mehr meckert.

Der Cast macht Sinn, weil, wenn du mal einen genaueren Blick auf die Zeile wirfst
Code:
itemElem.m_nItemNum	= (short)nItemNum;
Und auf jene:
Code:
				pUser->RemoveItem( (BYTE)( nId ), (short)( nItemNum ) );
				UpdateGuildBank( pGuild, GUILD_PUT_ITEM, 0, pUser->m_idPlayer, &itemElem, 0, (short)( nItemNum ) );
und dir nochmal den Link, den du voller Stolz gepostet hast, anguckst, siehst auch du, dass ein 32-Bit Integer ebenfalls zu einem Overflow führen kann. Wie schade, dass das ganze unsigniert ist und wie schade, dass uns am Ende einfach mal 16 Bit fehlen.

Mann muss bedenken das der FlyFF Source im Jahre 2000-2003~ programmiert wurde, zwischen drin updates hatte jedoch die Base jetzt schon älter als 10 fast 15 Jahre ist.

Ich verstehe nicht, was genau du casten möchtest, wenn du bereits einen short sendest. Der FlyFF Source ist wie mann bereits aus deinem Code-Schnipsel entnehmen kann auf einem short aufgebaut was die Item Anzahl angeht. Jetzt erkläre mir doch bitte noch einmal was man hier dran casten muss:

DPClient.h

PHP Code:


			
    void    SendPutItemGuildBank( BYTE nId, short ItemNum, BYTE p_Mode );

DPClient.cpp

PHP Code:


			
void CDPClient::SendPutItemGuildBank( BYTE nId, short ItemNum, BYTE p_Mode )
{
    BEFORESENDSOLE( ar, PACKETTYPE_PUTITEMGUILDBANK, DPID_UNKNOWN );
    ar << nId << ItemNum << p_Mode;
    SEND( ar, this, DPID_SERVERPLAYER );
}

DPSrvr.cpp

PHP Code:


			
void CDPSrvr::OnPutItemGuildBank( CAr & ar, DPID dpidCache, DPID dpidUser, LPBYTE lpBuf, u_long uBufSize )
{
#ifdef __DEBUG_LOG
    FILEOUT( "Debug.log", "CDPSrvr::OnPutItemGuildBank" );
#endif
    if( g_eLocal.GetState( ENABLE_GUILD_INVENTORY ) == FALSE )        
        return;

    BYTE nId, mode;
    short nItemNum;

    ar >> nId >> nItemNum >> mode;

    CUser* pUser    = g_UserMng.GetUser( dpidCache, dpidUser );    
    if( IsValidObj( pUser ) )
    {

#if __VER >= 15 // __GUILD_HOUSE
        if( !pUser->GetWorld() || !GuildHouseMng->IsGuildHouse( pUser->GetWorld()->GetID() ) )
#endif // __GUILD_HOUSE
            if( !CNpcChecker::GetInstance()->IsCloseNpc( MMI_GUILDBANKING, pUser->GetWorld(), pUser->GetPos() ) )
                return;

        if( mode == 0 )
            return;

        if( pUser->m_dwAuthorization >= AUTH_GAMEMASTER && pUser->m_dwAuthorization < AUTH_GAMEMASTER3 )
        {
            pUser->AddText( "Du hast kein Recht das zu tun." );
            return;
        }

        CItemElem* pItemElem = pUser->m_Inventory.GetAtId( nId );        
        if( IsUsableItem( pItemElem ) == FALSE )
            return;

        if( pItemElem->IsQuest() )
            return;
        
        if( pItemElem->IsBinds() )
            return;

        if( pUser->IsUsing( pItemElem ) )
        {
            pUser->AddDefinedText( TID_GAME_CANNOT_DO_USINGITEM );
            return;
        }

        ItemProp* pProp    = pItemElem->GetProp();
        if( pProp->dwParts == PARTS_RIDE && pProp->dwItemJob == JOB_VAGRANT )
            return;

        if( pUser->m_Inventory.IsEquip( nId ) )
        {
            pUser->AddDefinedText( TID_GAME_EQUIPTRADE, "" );
            return;
        }

        if( pItemElem->IsCharged() )
            return;
    
        if( nItemNum > pItemElem->m_nItemNum )
            nItemNum = pItemElem->m_nItemNum;
        if( nItemNum < 1 )
            nItemNum = 1;

        CGuild*    pGuild = pUser->GetGuild();
        if( pGuild )
        {
            CItemElem itemElem;
            itemElem    = *pItemElem;
            itemElem.m_nItemNum    = nItemNum;
            if ( pGuild->m_GuildBank.Add( &itemElem ) )
            {
                LogItemInfo aLogItem;
                aLogItem.Action = "W";
                aLogItem.SendName = pUser->GetName();
                aLogItem.RecvName = "GUILDBANK";
                aLogItem.WorldId = pUser->GetWorld()->GetID();
                aLogItem.Gold = aLogItem.Gold2 = pUser->GetGold();
                OnLogItem( aLogItem, &itemElem, nItemNum );
                OnLogItem( aLogItem, &itemElem, nItemNum );
                pUser->RemoveItem( (BYTE)( nId ), nItemNum  );
                UpdateGuildBank( pGuild, GUILD_PUT_ITEM, 0, pUser->m_idPlayer, &itemElem, 0, nItemNum );
                pUser->AddPutItemGuildBank( &itemElem );
                g_UserMng.AddPutItemElem( pUser, &itemElem );
            }
            else
            {
                pUser->AddDefinedText( TID_GAME_GUILDBANKFULL, "" );        
            }
        }
    }
}

Kann nicht genau nachvollziehen wo genau du nun casten möchtest.

Vielleicht solltest du dir noch einmal den FlyFF Source anschauen, vielleicht fällt es dir dann auf.

~~WurstbrotQT~~ · 01/19/2014, 15:18

Quote:

Originally Posted by Мentus

Mann muss bedenken das der FlyFF Source im Jahre 2000-2003~ programmiert wurde, zwischen drin updates hatte jedoch die Base jetzt schon älter als 10 fast 15 Jahre ist.

Ich verstehe nicht, was genau du casten möchtest, wenn du bereits einen short sendest. Der FlyFF Source ist wie mann bereits aus deinem Code-Schnipsel entnehmen kann auf einem short aufgebaut was die Item Anzahl angeht. Jetzt erkläre mir doch bitte noch einmal was man hier dran casten muss:

DPClient.h

PHP Code:

    void    SendPutItemGuildBank( BYTE nId, short ItemNum, BYTE p_Mode );

DPClient.cpp

PHP Code:

void CDPClient::SendPutItemGuildBank( BYTE nId, short ItemNum, BYTE p_Mode ) {     BEFORESENDSOLE( ar, PACKETTYPE_PUTITEMGUILDBANK, DPID_UNKNOWN );     ar << nId << ItemNum << p_Mode;     SEND( ar, this, DPID_SERVERPLAYER ); }

DPSrvr.cpp

PHP Code:

void CDPSrvr::OnPutItemGuildBank( CAr & ar, DPID dpidCache, DPID dpidUser, LPBYTE lpBuf, u_long uBufSize ) { #ifdef __DEBUG_LOG     FILEOUT( "Debug.log", "CDPSrvr::OnPutItemGuildBank" ); #endif     if( g_eLocal.GetState( ENABLE_GUILD_INVENTORY ) == FALSE )                 return;     BYTE nId, mode;     short nItemNum;     ar >> nId >> nItemNum >> mode;     CUser* pUser    = g_UserMng.GetUser( dpidCache, dpidUser );         if( IsValidObj( pUser ) )     { #if __VER >= 15 // __GUILD_HOUSE         if( !pUser->GetWorld() || !GuildHouseMng->IsGuildHouse( pUser->GetWorld()->GetID() ) ) #endif // __GUILD_HOUSE             if( !CNpcChecker::GetInstance()->IsCloseNpc( MMI_GUILDBANKING, pUser->GetWorld(), pUser->GetPos() ) )                 return;         if( mode == 0 )             return;         if( pUser->m_dwAuthorization >= AUTH_GAMEMASTER && pUser->m_dwAuthorization < AUTH_GAMEMASTER3 )         {             pUser->AddText( "Du hast kein Recht das zu tun." );             return;         }         CItemElem* pItemElem = pUser->m_Inventory.GetAtId( nId );                 if( IsUsableItem( pItemElem ) == FALSE )             return;         if( pItemElem->IsQuest() )             return;                  if( pItemElem->IsBinds() )             return;         if( pUser->IsUsing( pItemElem ) )         {             pUser->AddDefinedText( TID_GAME_CANNOT_DO_USINGITEM );             return;         }         ItemProp* pProp    = pItemElem->GetProp();         if( pProp->dwParts == PARTS_RIDE && pProp->dwItemJob == JOB_VAGRANT )             return;         if( pUser->m_Inventory.IsEquip( nId ) )         {             pUser->AddDefinedText( TID_GAME_EQUIPTRADE, "" );             return;         }         if( pItemElem->IsCharged() )             return;              if( nItemNum > pItemElem->m_nItemNum )             nItemNum = pItemElem->m_nItemNum;         if( nItemNum < 1 )             nItemNum = 1;         CGuild*    pGuild = pUser->GetGuild();         if( pGuild )         {             CItemElem itemElem;             itemElem    = *pItemElem;             itemElem.m_nItemNum    = nItemNum;             if ( pGuild->m_GuildBank.Add( &itemElem ) )             {                 LogItemInfo aLogItem;                 aLogItem.Action = "W";                 aLogItem.SendName = pUser->GetName();                 aLogItem.RecvName = "GUILDBANK";                 aLogItem.WorldId = pUser->GetWorld()->GetID();                 aLogItem.Gold = aLogItem.Gold2 = pUser->GetGold();                 OnLogItem( aLogItem, &itemElem, nItemNum );                 OnLogItem( aLogItem, &itemElem, nItemNum );                 pUser->RemoveItem( (BYTE)( nId ), nItemNum  );                 UpdateGuildBank( pGuild, GUILD_PUT_ITEM, 0, pUser->m_idPlayer, &itemElem, 0, nItemNum );                 pUser->AddPutItemGuildBank( &itemElem );                 g_UserMng.AddPutItemElem( pUser, &itemElem );             }             else             {                 pUser->AddDefinedText( TID_GAME_GUILDBANKFULL, "" );                     }         }     } }

Kann nicht genau nachvollziehen wo genau du nun casten möchtest.

Vielleicht solltest du dir noch einmal den FlyFF Source anschauen, vielleicht fällt es dir dann auf.

Da stellt sich mir die Frage, was genau du der Funktion

Code:

 CDPClient::SendPutItemGuildBank

übergibst und wo die aufgerufen wird.
Es wird sicherlich default kein short übergeben, sondern int

Code:

g_DPlay.SendPutItemGuildBank( (BYTE)( m_Shortcut.m_dwId ), nCost, 1 );

Und zum Thema Cast und deinem Buch, ist mir eben noch eingefallen, steht in deinem Buch auch, wie man Polymorphie und späte Bindung ohne Upcast realisieren kann?

Aber wir wollen das Thema jetzt auch nicht auseinanderdiskutieren wie blöde, ich halte beide Lösungen für valide und für beide werden Casts benötigt, es sei denn du änderst die Item Struktur oder lässt es als Int stehen.

Jedoch muss ich sagen, dass mit Lethals Version auch besser gefällt, da es im Endeffekt nach der Abfrage egal ist, ob nun int in short gecastet wurde oder ob bereits mit short überprüft wurde.

Und die Gschichte mit dem Flyff Source, ja, es stimmt, Objective C und C++ sind ein riesen Unterschied, aber es lohnt sich einfach nicht, neue Systeme in validem C++ zu implementieren. Das würde dem Source sämtliche gebliebene Struktur nehmen. In valid bzw native C++ wird nämlich eigentlich nicht mit etwaigen char Arrays gearbeitet wie im Source, aber lassen wir diese Diskussionen, letztendlich kommt es für mich auf die Funktionalität an.

Мentus · 01/19/2014, 15:57

Quote:
Originally Posted by WurstbrotQT
Da stellt sich mir die Frage, was genau du der Funktion
Code:
 CDPClient::SendPutItemGuildBank
übergibst und wo die aufgerufen wird.
Es wird sicherlich default kein short übergeben, sondern int
Code:
g_DPlay.SendPutItemGuildBank( (BYTE)( m_Shortcut.m_dwId ), nCost, 1 );
Und zum Thema Cast und deinem Buch, ist mir eben noch eingefallen, steht in deinem Buch auch, wie man Polymorphie und späte Bindung ohne Upcast realisieren kann?

Aber wir wollen das Thema jetzt auch nicht auseinanderdiskutieren wie blöde, ich halte beide Lösungen für valide und für beide werden Casts benötigt, es sei denn du änderst die Item Struktur oder lässt es als Int stehen.

Jedoch muss ich sagen, dass mit Lethals Version auch besser gefällt, da es im Endeffekt nach der Abfrage egal ist, ob nun int in short gecastet wurde oder ob bereits mit short überprüft wurde.

Und die Gschichte mit dem Flyff Source, ja, es stimmt, Objective C und C++ sind ein riesen Unterschied, aber es lohnt sich einfach nicht, neue Systeme in validem C++ zu implementieren. Das würde dem Source sämtliche gebliebene Struktur nehmen. In valid bzw native C++ wird nämlich eigentlich nicht mit etwaigen char Arrays gearbeitet wie im Source, aber lassen wir diese Diskussionen, letztendlich kommt es für mich auf die Funktionalität an.

Müsste ich nachschauen, hatte das Buch nur erwähnt weil ich dort mal was nachgeschlagen hatte.

Jedoch handelt es sich es hier eh nur um kleinere Unterschiede.
Wobei ich das mit dem im Client casten bevorzugen würde. Da würde z.b nie ein Dupe entstehen und darum geht es ja hier.

Aber dein Fix sollte es auch beheben nur dachte ich anfangs du machst den gleichen Fehler wie die Flyff Dev und prüfst den nicht "gecasteten" Wert. Das wäre halt wieder ne Dupe Möglichkeit.

Aber ansonsten sollte es eigentlich alles so passen.