some bugs maybe！

viaviaviavia
edited June 2013 in Bug report - Feature request
hi：

I found some bugs, i'll list them one by one at each floor.

Comments

  • viaviaviavia
    edited June 2013
    1.in codesrc
    enderorxShader.c
    orxSHADER *orxFASTCALL orxShader_Create()
{
...
 if((pstResult->pstParamValueBank != orxNULL)
    && (pstResult->pstParamBank != orxNULL))
    {
     ...
    }
else
    {
      /* Deletes bank */
      if(pstResult->pstParamValueBank != orxNULL)
      {
        orxBank_Delete(pstResult->pstParamValueBank);
      }

    ...
    }
...
}

    here,why just check for "pstParamValueBank " ,not for "pstParamBank"?
  • viaviaviavia
    edited June 2013
    2.in codesrc
    enderorxShader.c
    orxSTATUS orxFASTCALL orxShader_Delete(orxSHADER *_pstShader)
{
...
for(pstParamValue = (orxSHADER_PARAM_VALUE *)orxLinkList_GetFirst(&(_pstShader->stParamValueList));
          pstParamValue != orxNULL;
          pstParamValue = (orxSHADER_PARAM_VALUE *)orxLinkList_GetNext(&(pstParamValue->stNode)))
      {
       ...
      if((pstParamValue->pstValue != orxNULL) && (pstParamValue->s32Index <= 0))
          {
           ...
          }
...
}


    here,"pstParamValue->s32Index <= 0" is right?
  • viaviaviavia
    edited June 2013
    3.in codesrcanimorxAnim.c:
    orxANIM *orxFASTCALL orxAnim_Create(orxU32 _u32Flags, orxU32 _u32KeyNumber, orxU32 _u32EventNumber)
{
...
pstAnim->astKeyList = (orxANIM_KEY *)orxMemory_Allocate(_u32KeyNumber * sizeof(orxANIM_KEY), orxMEMORY_TYPE_MAIN);
...
pstAnim->astEventList = (orxANIM_CUSTOM_EVENT *)orxMemory_Allocate(_u32EventNumber * sizeof(orxANIM_CUSTOM_EVENT), orxMEMORY_TYPE_MAIN);
...
}

    here,i see u allocate memory for two arrays, but i don't see any "orxMemory_Free" for them when delete.
  • viaviaviavia
    edited June 2013
    4.in codesrcobjectorxTimeLine.c
    
static orxINLINE orxTIMELINE_TRACK *orxTimeLine_CreateTrack(const orxSTRING _zTrackID)
{
...
 u32KeyCounter = orxConfig_GetKeyCounter();

    /* Valid? */
  if(u32KeyCounter > 0)
  {

   pstResult = (orxTIMELINE_TRACK *)orxMemory_Allocate(sizeof(orxTIMELINE_TRACK) + (u32EventCounter * sizeof(orxTIMELINE_TRACK_EVENT)), orxMEMORY_TYPE_MAIN);

     if(pstResult != orxNULL)
      {
      ...
      }
     else
      {
        /* Logs message */
        orxDEBUG_PRINT(orxDEBUG_LEVEL_OBJECT, "Couldn't create TimeLine track [%s]: config section is empty.", _zTrackID);
      }

  }
  else
  {
      /* Logs message */
      orxDEBUG_PRINT(orxDEBUG_LEVEL_OBJECT, "Couldn't create TimeLine track [%s]: memory allocation failure.", _zTrackID);
  }

...
}


    the two "orxDEBUG_PRINT" should exchange for each other!
  • viaviaviavia
    edited June 2013
    5.in codesrcobjectorxObject.c:
    orxOBOX *orxFASTCALL orxObject_GetBoundingBox(const orxOBJECT *_pstObject, orxOBOX *_pstBoundingBox)
{
...
  else
  {
    /* Updates result */
    pstResult = orxNULL;

    /* Cleans it */
    orxMemory_Zero(_pstBoundingBox, sizeof(orxAABOX));
  }
...
}

    here,"pstBoundingBox" is a "orxOBOX *" ,"sizeof(orxAABOX)" is right???
  • viaviaviavia
    edited June 2013
    6.in codesrcobjectorxObject.c:
    orxBANK *orxFASTCALL orxObject_CreateNeighborList(const orxOBOX *_pstCheckBox)
{
...
    for(u32Counter = 0, pstObject = orxOBJECT(orxStructure_GetFirst(orxSTRUCTURE_ID_OBJECT));
        (u32Counter < orxOBJECT_KU32_NEIGHBOR_LIST_SIZE) && (pstObject != orxNULL);
        pstObject = orxOBJECT(orxStructure_GetNext(pstObject)), u32Counter++)
    {
     if(orxObject_GetBoundingBox(pstObject, &stObjectBox) != orxNULL)
      {         
        if(orxOBox_ZAlignedTestIntersection(_pstCheckBox, &stObjectBox) != orxFALSE)
        {
        ...
         if(ppstObject != orxNULL)
          {
            /* Adds object */
            *ppstObject = pstObject;
          }   
        }
      }
    }
...
}


    i think "u32Counter" is the number of available neighbors in the"NEIGHBOR_LIST" array ,so "u32Counter++" should be in 
    if(ppstObject != orxNULL)
{
  /* Adds object */
  *ppstObject = pstObject;
}
  • viaviaviavia
    edited June 2013
    below is some no-harm bugs, maybe not bugs.


    7.in codesrc
    ender

    Is type name "orxSOUNDPOINTER_HOLDER" a typo? should be "orxSHADERPOINTER_HOLDER"?

    ah ,it's just a small one, i just say it ,need not to change.
  • viaviaviavia
    edited June 2013
    8.in codesrcanimorxAnimSet.c:
    static orxU32 orxFASTCALL orxAnimSet_ComputeNextAnim(orxANIMSET_LINK_TABLE *_pstLinkTable, orxU32 _u32SrcAnim, orxU32 _u32DstAnim, orxBOOL _bSimulate)
{
...
if(u32Result != orxU32_UNDEFINED)
  {
   ...
   if(u32Loop <= 1)
        {
          /* Updates flags */
          orxAnimSet_SetLinkTableFlag(_pstLinkTable, orxANIMSET_KU32_LINK_TABLE_FLAG_DIRTY, orxANIMSET_KU32_LINK_TABLE_FLAG_NONE);
        }
  }
}


    First,i'm sorry , i'm not sure if you've got any change that would affect my suggestion,because it've been some time past since i read this file.and i've forgotten some details of it.


    here,u want: if the loop get its end ,set the link table to "dirty" to recompute it. But in my opinion when i read it ,i found the "loop" state would not cause function "orxAnimSet_ComputeLinkTable" to change anything.

    And of course ,the "loop" is part of the link table , and it should be recomputered in theory ,but just for now it runs for nothing.


    maybe i'am wrong.haha.
  • viaviaviavia
    edited June 2013
    and there's two more .but i've forgotten some details now as i read it some time before. So i'm not quite sure ,and the two source files is large ,so i'll take some time to review them,and then post them if neccessory.


    thanks for your time!
  • iarwainiarwain
    edited June 2013
    Hi viavia,

    Thanks for all that sleuth work, much appreciated. I'll go over them tomorrow. Most of them sounds legit from here. I'll post here again when I'm done. :)
  • viaviaviavia
    edited June 2013
    u are welcome,take your time.
  • iarwainiarwain
    edited June 2013
    Hi viavia,

    I fixed all except #8: it's actually a feature that's only partly implemented. You're right that when saying that the way it is right now, flagging the table as dirty won't have any impact, however at some point loop exhaustion is supposed to trigger a complete link re-evaluation (which it does), but that will take into account the available loop counter. That's the missing part there. I guess I could add a comment/todo to make it more obvious. :)
Sign In or Register to comment.