From 05240ccbe57f89619f1fea4c8610728e3c5ecc5c Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Tue, 11 Apr 2017 21:48:41 +0200 Subject: [PATCH] - fixed redundant reallocation of constructable meta fields. - some optimization of access to OwnedStates in old DECORATE. - consolidate all places that print a state name into a subfunction. - allocate states from the ClassDataAllocator memory arena. States do not need to be freed separately from the rest of the static class data. --- src/dobjtype.cpp | 26 +++++- src/dobjtype.h | 1 + src/info.cpp | 4 - src/info.h | 1 + src/p_actionfunctions.cpp | 11 +-- src/p_mobj.cpp | 3 +- src/p_pspr.cpp | 15 ++-- src/p_states.cpp | 23 ++++-- src/scripting/decorate/olddecorations.cpp | 97 ++++++++++++----------- src/scripting/thingdef.cpp | 10 +-- 10 files changed, 106 insertions(+), 85 deletions(-) diff --git a/src/dobjtype.cpp b/src/dobjtype.cpp index b4fd9835d0..c3932e068e 100644 --- a/src/dobjtype.cpp +++ b/src/dobjtype.cpp @@ -2888,6 +2888,12 @@ void PClass::StaticShutdown () p.PendingWeapon = nullptr; } + // This must be done before the type table is taken down. + for (auto cls : AllClasses) + { + Printf("Processing %s\n", cls->TypeName.GetChars()); + cls->DestroyMeta(cls->Meta); + } // Unless something went wrong, anything left here should be class and type objects only, which do not own any scripts. bShutdown = true; TypeTable.Clear(); @@ -3162,8 +3168,6 @@ void PClass::InitializeSpecials(void *addr, void *defaults, TArrayDestroyMeta(addr); + for (auto tao : MetaInits) + { + tao.first->DestroyValue((uint8_t *)addr + tao.second); + } +} + //========================================================================== // // PClass :: Derive @@ -3265,7 +3286,6 @@ void PClass::InitializeDefaults() // Copy parent values from the parent defaults. assert(ParentClass != nullptr); if (Defaults != nullptr) ParentClass->InitializeSpecials(Defaults, ParentClass->Defaults, &PClass::SpecialInits); - if (Meta != nullptr) ParentClass->InitializeSpecials(Meta, ParentClass->Meta, &PClass::MetaInits); for (const PField *field : Fields) { if (!(field->Flags & VARF_Native) && !(field->Flags & VARF_Meta)) diff --git a/src/dobjtype.h b/src/dobjtype.h index bf6e9feb14..de364c4f8f 100644 --- a/src/dobjtype.h +++ b/src/dobjtype.h @@ -634,6 +634,7 @@ public: void BuildArrayPointers(); void InitMeta(); void DestroySpecials(void *addr); + void DestroyMeta(void *addr); const PClass *NativeClass() const; // Returns true if this type is an ancestor of (or same as) the passed type. diff --git a/src/info.cpp b/src/info.cpp index f32baebc71..23c7fb7ccf 100644 --- a/src/info.cpp +++ b/src/info.cpp @@ -318,10 +318,6 @@ PClassActor::PClassActor() PClassActor::~PClassActor() { - if (OwnedStates != NULL) - { - delete[] OwnedStates; - } if (DamageFactors != NULL) { delete DamageFactors; diff --git a/src/info.h b/src/info.h index 575f9318d7..466e925cb5 100644 --- a/src/info.h +++ b/src/info.h @@ -177,6 +177,7 @@ public: bool CallAction(AActor *self, AActor *stateowner, FStateParamInfo *stateinfo, FState **stateret); static PClassActor *StaticFindStateOwner (const FState *state); static PClassActor *StaticFindStateOwner (const FState *state, PClassActor *info); + static FString StaticGetStateName(const FState *state); static FRandom pr_statetics; }; diff --git a/src/p_actionfunctions.cpp b/src/p_actionfunctions.cpp index 97be1b1082..657a89bb30 100644 --- a/src/p_actionfunctions.cpp +++ b/src/p_actionfunctions.cpp @@ -130,8 +130,7 @@ bool AStateProvider::CallStateChain (AActor *actor, FState *state) { if (!(state->UseFlags & SUF_ITEM)) { - auto so = FState::StaticFindStateOwner(state); - Printf(TEXTCOLOR_RED "State %s.%d not flagged for use in CustomInventory state chains.\n", so->TypeName.GetChars(), int(state - so->OwnedStates)); + Printf(TEXTCOLOR_RED "State %s not flagged for use in CustomInventory state chains.\n", FState::StaticGetStateName(state)); return false; } @@ -144,8 +143,8 @@ bool AStateProvider::CallStateChain (AActor *actor, FState *state) { // If an unsafe function (i.e. one that accesses user variables) is being detected, print a warning once and remove the bogus function. We may not call it because that would inevitably crash. auto owner = FState::StaticFindStateOwner(state); - Printf(TEXTCOLOR_RED "Unsafe state call in state %s.%d to %s which accesses user variables. The action function has been removed from this state\n", - owner->TypeName.GetChars(), int(state - owner->OwnedStates), state->ActionFunc->PrintableName.GetChars()); + Printf(TEXTCOLOR_RED "Unsafe state call in state %s to %s which accesses user variables. The action function has been removed from this state\n", + FState::StaticGetStateName(state), state->ActionFunc->PrintableName.GetChars()); state->ActionFunc = nullptr; } @@ -188,9 +187,7 @@ bool AStateProvider::CallStateChain (AActor *actor, FState *state) catch (CVMAbortException &err) { err.MaybePrintMessage(); - auto owner = FState::StaticFindStateOwner(state); - int offs = int(state - owner->OwnedStates); - err.stacktrace.AppendFormat("Called from state %s.%d in inventory state chain in %s\n", owner->TypeName.GetChars(), offs, GetClass()->TypeName.GetChars()); + err.stacktrace.AppendFormat("Called from state %s in inventory state chain in %s\n", FState::StaticGetStateName(state), GetClass()->TypeName.GetChars()); throw; } diff --git a/src/p_mobj.cpp b/src/p_mobj.cpp index 6a543f091d..0632ba20b2 100644 --- a/src/p_mobj.cpp +++ b/src/p_mobj.cpp @@ -632,8 +632,7 @@ bool AActor::SetState (FState *newstate, bool nofunction) } if (!(newstate->UseFlags & SUF_ACTOR)) { - auto so = FState::StaticFindStateOwner(newstate); - Printf(TEXTCOLOR_RED "State %s.%d in %s not flagged for use as an actor sprite\n", so->TypeName.GetChars(), int(newstate - so->OwnedStates), GetClass()->TypeName.GetChars()); + Printf(TEXTCOLOR_RED "State %s in %s not flagged for use as an actor sprite\n", FState::StaticGetStateName(newstate), GetClass()->TypeName.GetChars()); state = nullptr; Destroy(); return false; diff --git a/src/p_pspr.cpp b/src/p_pspr.cpp index c0e8a3a4b7..959dbf2202 100644 --- a/src/p_pspr.cpp +++ b/src/p_pspr.cpp @@ -350,8 +350,7 @@ void DPSprite::SetState(FState *newstate, bool pending) if (!(newstate->UseFlags & (SUF_OVERLAY|SUF_WEAPON))) // Weapon and overlay are mostly the same, the main difference is that weapon states restrict the self pointer to class Actor. { - auto so = FState::StaticFindStateOwner(newstate); - Printf(TEXTCOLOR_RED "State %s.%d not flagged for use in overlays or weapons\n", so->TypeName.GetChars(), int(newstate - so->OwnedStates)); + Printf(TEXTCOLOR_RED "State %s not flagged for use in overlays or weapons\n", FState::StaticGetStateName(newstate)); State = nullptr; Destroy(); return; @@ -360,8 +359,7 @@ void DPSprite::SetState(FState *newstate, bool pending) { if (Caller->IsKindOf(NAME_Weapon)) { - auto so = FState::StaticFindStateOwner(newstate); - Printf(TEXTCOLOR_RED "State %s.%d not flagged for use in weapons\n", so->TypeName.GetChars(), int(newstate - so->OwnedStates)); + Printf(TEXTCOLOR_RED "State %s.%d not flagged for use in weapons\n", FState::StaticGetStateName(newstate)); State = nullptr; Destroy(); return; @@ -414,9 +412,8 @@ void DPSprite::SetState(FState *newstate, bool pending) if (newstate->ActionFunc != nullptr && newstate->ActionFunc->Unsafe) { // If an unsafe function (i.e. one that accesses user variables) is being detected, print a warning once and remove the bogus function. We may not call it because that would inevitably crash. - auto owner = FState::StaticFindStateOwner(newstate); - Printf(TEXTCOLOR_RED "Unsafe state call in state %s.%d to %s which accesses user variables. The action function has been removed from this state\n", - owner->TypeName.GetChars(), int(newstate - owner->OwnedStates), newstate->ActionFunc->PrintableName.GetChars()); + Printf(TEXTCOLOR_RED "Unsafe state call in state %sd to %s which accesses user variables. The action function has been removed from this state\n", + FState::StaticGetStateName(newstate), newstate->ActionFunc->PrintableName.GetChars()); newstate->ActionFunc = nullptr; } if (newstate->CallAction(Owner->mo, Caller, &stp, &nextstate)) @@ -1512,11 +1509,11 @@ void P_SetSafeFlash(AWeapon *weapon, player_t *player, FState *flashstate, int i PClassActor *cls = weapon->GetClass(); while (cls != RUNTIME_CLASS(AWeapon)) { - if (flashstate >= cls->OwnedStates && flashstate < cls->OwnedStates + cls->NumOwnedStates) + if (cls->OwnsState(flashstate)) { // The flash state belongs to this class. // Now let's check if the actually wanted state does also - if (flashstate + index < cls->OwnedStates + cls->NumOwnedStates) + if (cls->OwnsState(flashstate + index)) { // we're ok so set the state P_SetPsprite(player, PSP_FLASH, flashstate + index, true); diff --git a/src/p_states.cpp b/src/p_states.cpp index 762a768965..731cbed9c3 100644 --- a/src/p_states.cpp +++ b/src/p_states.cpp @@ -96,8 +96,7 @@ PClassActor *FState::StaticFindStateOwner (const FState *state) for (unsigned int i = 0; i < PClassActor::AllActorClasses.Size(); ++i) { PClassActor *info = PClassActor::AllActorClasses[i]; - if (state >= info->OwnedStates && - state < info->OwnedStates + info->NumOwnedStates) + if (info->OwnsState(state)) { return info; } @@ -117,8 +116,7 @@ PClassActor *FState::StaticFindStateOwner (const FState *state, PClassActor *inf { while (info != NULL) { - if (state >= info->OwnedStates && - state < info->OwnedStates + info->NumOwnedStates) + if (info->OwnsState(state)) { return info; } @@ -127,6 +125,16 @@ PClassActor *FState::StaticFindStateOwner (const FState *state, PClassActor *inf return NULL; } +//========================================================================== +// +// +//========================================================================== + +FString FState::StaticGetStateName(const FState *state) +{ + auto so = FState::StaticFindStateOwner(state); + return FStringf("%s.%d", so->TypeName.GetChars(), int(state - so->OwnedStates)); +} //========================================================================== // @@ -1000,7 +1008,7 @@ int FStateDefinitions::FinishStates(PClassActor *actor, AActor *defaults) if (count > 0) { - FState *realstates = new FState[count]; + FState *realstates = (FState*)ClassDataAllocator.Alloc(count * sizeof(FState)); int i; memcpy(realstates, &StateArray[0], count*sizeof(FState)); @@ -1071,8 +1079,7 @@ void DumpStateHelper(FStateLabels *StateList, const FString &prefix) } else { - Printf(PRINT_LOG, "%s%s: %s.%d\n", prefix.GetChars(), StateList->Labels[i].Label.GetChars(), - owner->TypeName.GetChars(), int(StateList->Labels[i].State - owner->OwnedStates)); + Printf(PRINT_LOG, "%s%s: %s\n", prefix.GetChars(), StateList->Labels[i].Label.GetChars(), FState::StaticGetStateName(StateList->Labels[i].State)); } } if (StateList->Labels[i].Children != NULL) @@ -1124,7 +1131,7 @@ DEFINE_ACTION_FUNCTION(FState, DistanceTo) { // Safely calculate the distance between two states. auto o1 = FState::StaticFindStateOwner(self); - if (other >= o1->OwnedStates && other < o1->OwnedStates + o1->NumOwnedStates) retv = int(other - self); + if (o1->OwnsState(other)) retv = int(other - self); } ACTION_RETURN_INT(retv); } diff --git a/src/scripting/decorate/olddecorations.cpp b/src/scripting/decorate/olddecorations.cpp index 9ecf8c2a02..b9c618971f 100644 --- a/src/scripting/decorate/olddecorations.cpp +++ b/src/scripting/decorate/olddecorations.cpp @@ -86,7 +86,7 @@ static const char *RenderStyles[] = "STYLE_Translucent", "STYLE_Add", //"STYLE_Shaded", - NULL + nullptr }; // CODE -------------------------------------------------------------------- @@ -158,66 +158,69 @@ void ParseOldDecoration(FScanner &sc, EDefinitionType def, PNamespace *ns) type->NumOwnedStates += 1; } - type->OwnedStates = new FState[type->NumOwnedStates]; - SaveStateSourceLines(type->OwnedStates, SourceLines); - memcpy (type->OwnedStates, &StateArray[0], type->NumOwnedStates * sizeof(type->OwnedStates[0])); + FState *states; + states = type->OwnedStates = (FState*)ClassDataAllocator.Alloc(type->NumOwnedStates * sizeof(FState)); + SaveStateSourceLines(states, SourceLines); + memcpy (states, &StateArray[0], type->NumOwnedStates * sizeof(states[0])); if (type->NumOwnedStates == 1) { - type->OwnedStates->Tics = -1; - type->OwnedStates->TicRange = 0; - type->OwnedStates->Misc1 = 0; + states->Tics = -1; + states->TicRange = 0; + states->Misc1 = 0; } else { size_t i; + // auto // Spawn states loop endlessly for (i = extra.SpawnStart; i < extra.SpawnEnd-1; ++i) { - type->OwnedStates[i].NextState = &type->OwnedStates[i+1]; + states[i].NextState = &states[i+1]; } - type->OwnedStates[i].NextState = &type->OwnedStates[extra.SpawnStart]; + states[i].NextState = &states[extra.SpawnStart]; // Death states are one-shot and freeze on the final state if (extra.DeathEnd != 0) { for (i = extra.DeathStart; i < extra.DeathEnd-1; ++i) { - type->OwnedStates[i].NextState = &type->OwnedStates[i+1]; + states[i].NextState = &states[i+1]; } + FState *state = &states[i]; if (extra.bDiesAway || def == DEF_Projectile) { - type->OwnedStates[i].NextState = NULL; + state->NextState = nullptr; } else { - type->OwnedStates[i].Tics = -1; - type->OwnedStates[i].TicRange = 0; - type->OwnedStates[i].Misc1 = 0; + state->Tics = -1; + state->TicRange = 0; + state->Misc1 = 0; } if (def == DEF_Projectile) { if (extra.bExplosive) { - type->OwnedStates[extra.DeathStart].SetAction("A_Explode"); + states[extra.DeathStart].SetAction("A_Explode"); } } else { // The first frame plays the death sound and // the second frame makes it nonsolid. - type->OwnedStates[extra.DeathStart].SetAction("A_Scream"); + states[extra.DeathStart].SetAction("A_Scream"); if (extra.bSolidOnDeath) { } else if (extra.DeathStart + 1 < extra.DeathEnd) { - type->OwnedStates[extra.DeathStart+1].SetAction("A_NoBlocking"); + states[extra.DeathStart+1].SetAction("A_NoBlocking"); } else { - type->OwnedStates[extra.DeathStart].SetAction("A_ScreamAndUnblock"); + states[extra.DeathStart].SetAction("A_ScreamAndUnblock"); } if (extra.DeathHeight == 0) @@ -226,7 +229,7 @@ void ParseOldDecoration(FScanner &sc, EDefinitionType def, PNamespace *ns) } ((AActor*)(type->Defaults))->FloatVar("DeathHeight") = extra.DeathHeight; } - bag.statedef.SetStateLabel("Death", &type->OwnedStates[extra.DeathStart]); + bag.statedef.SetStateLabel("Death", &states[extra.DeathStart]); } // Burn states are the same as death states, except they can optionally terminate @@ -234,38 +237,39 @@ void ParseOldDecoration(FScanner &sc, EDefinitionType def, PNamespace *ns) { for (i = extra.FireDeathStart; i < extra.FireDeathEnd-1; ++i) { - type->OwnedStates[i].NextState = &type->OwnedStates[i+1]; + states[i].NextState = &states[i+1]; } + FState *state = &states[i]; if (extra.bBurnAway) { - type->OwnedStates[i].NextState = NULL; + state->NextState = nullptr; } else { - type->OwnedStates[i].Tics = -1; - type->OwnedStates[i].TicRange = 0; - type->OwnedStates[i].Misc1 = 0; + state->Tics = -1; + state->TicRange = 0; + state->Misc1 = 0; } // The first frame plays the burn sound and // the second frame makes it nonsolid. - type->OwnedStates[extra.FireDeathStart].SetAction("A_ActiveSound"); + states[extra.FireDeathStart].SetAction("A_ActiveSound"); if (extra.bSolidOnBurn) { } else if (extra.FireDeathStart + 1 < extra.FireDeathEnd) { - type->OwnedStates[extra.FireDeathStart+1].SetAction("A_NoBlocking"); + states[extra.FireDeathStart+1].SetAction("A_NoBlocking"); } else { - type->OwnedStates[extra.FireDeathStart].SetAction("A_ActiveAndUnblock"); + states[extra.FireDeathStart].SetAction("A_ActiveAndUnblock"); } if (extra.BurnHeight == 0) extra.BurnHeight = ((AActor*)(type->Defaults))->Height; ((AActor*)(type->Defaults))->FloatVar("BurnHeight") = extra.BurnHeight; - bag.statedef.SetStateLabel("Burn", &type->OwnedStates[extra.FireDeathStart]); + bag.statedef.SetStateLabel("Burn", &states[extra.FireDeathStart]); } // Ice states are similar to burn and death, except their final frame enters @@ -274,21 +278,22 @@ void ParseOldDecoration(FScanner &sc, EDefinitionType def, PNamespace *ns) { for (i = extra.IceDeathStart; i < extra.IceDeathEnd-1; ++i) { - type->OwnedStates[i].NextState = &type->OwnedStates[i+1]; + states[i].NextState = &states[i+1]; } - type->OwnedStates[i].NextState = &type->OwnedStates[type->NumOwnedStates-1]; - type->OwnedStates[i].Tics = 5; - type->OwnedStates[i].TicRange = 0; - type->OwnedStates[i].Misc1 = 0; - type->OwnedStates[i].SetAction("A_FreezeDeath"); + FState *state = &states[i]; + state->NextState = &states[type->NumOwnedStates-1]; + state->Tics = 5; + state->TicRange = 0; + state->Misc1 = 0; + state->SetAction("A_FreezeDeath"); i = type->NumOwnedStates - 1; - type->OwnedStates[i].NextState = &type->OwnedStates[i]; - type->OwnedStates[i].Tics = 1; - type->OwnedStates[i].TicRange = 0; - type->OwnedStates[i].Misc1 = 0; - type->OwnedStates[i].SetAction("A_FreezeDeathChunks"); - bag.statedef.SetStateLabel("Ice", &type->OwnedStates[extra.IceDeathStart]); + state->NextState = &states[i]; + state->Tics = 1; + state->TicRange = 0; + state->Misc1 = 0; + state->SetAction("A_FreezeDeathChunks"); + bag.statedef.SetStateLabel("Ice", &states[extra.IceDeathStart]); } else if (extra.bGenericIceDeath) { @@ -303,7 +308,7 @@ void ParseOldDecoration(FScanner &sc, EDefinitionType def, PNamespace *ns) { ((AActor *)(type->Defaults))->flags |= MF_DROPOFF|MF_MISSILE; } - bag.statedef.SetStateLabel("Spawn", &type->OwnedStates[extra.SpawnStart]); + bag.statedef.SetStateLabel("Spawn", &states[extra.SpawnStart]); bag.statedef.InstallStates (type, ((AActor *)(type->Defaults))); } @@ -558,11 +563,11 @@ static void ParseInsideDecoration (Baggage &bag, AActor *defaults, } else if (sc.String[0] != '*') { - HandleActorFlag(sc, bag, sc.String, NULL, '+'); + HandleActorFlag(sc, bag, sc.String, nullptr, '+'); } else { - sc.ScriptError (NULL); + sc.ScriptError (nullptr); } sc.MustGetString (); } @@ -621,7 +626,7 @@ static void ParseSpriteFrames (PClassActor *info, TArray &states, TArray memset (&state, 0, sizeof(state)); state.UseFlags = info->DefaultStateUsage; - while (token != NULL) + while (token != nullptr) { // Skip leading white space while (*token == ' ') @@ -631,7 +636,7 @@ static void ParseSpriteFrames (PClassActor *info, TArray &states, TArray bool firstState = true; char *colon = strchr (token, ':'); - if (colon != NULL) + if (colon != nullptr) { char *stop; @@ -682,7 +687,7 @@ static void ParseSpriteFrames (PClassActor *info, TArray &states, TArray SourceLines.Push(sc); } - token = strtok (NULL, ",\t\n\r"); + token = strtok (nullptr, ",\t\n\r"); } } diff --git a/src/scripting/thingdef.cpp b/src/scripting/thingdef.cpp index a8503e64e0..0cb25b1c26 100644 --- a/src/scripting/thingdef.cpp +++ b/src/scripting/thingdef.cpp @@ -282,9 +282,8 @@ static void CheckForUnsafeStates(PClassActor *obj) if (state->ActionFunc && state->ActionFunc->Unsafe) { // If an unsafe function (i.e. one that accesses user variables) is being detected, print a warning once and remove the bogus function. We may not call it because that would inevitably crash. - auto owner = FState::StaticFindStateOwner(state); - GetStateSource(state).Message(MSG_ERROR, TEXTCOLOR_RED "Unsafe state call in state %s.%d which accesses user variables, reached by %s.%s.\n", - owner->TypeName.GetChars(), int(state - owner->OwnedStates), obj->TypeName.GetChars(), FName(*test).GetChars()); + GetStateSource(state).Message(MSG_ERROR, TEXTCOLOR_RED "Unsafe state call in state %s which accesses user variables, reached by %s.%s.\n", + FState::StaticGetStateName(state), obj->TypeName.GetChars(), FName(*test).GetChars()); } state = state->NextState; } @@ -308,9 +307,8 @@ static void CheckLabel(PClassActor *obj, FStateLabel *slb, int useflag, FName st { if (!(state->UseFlags & useflag)) { - auto owner = FState::StaticFindStateOwner(state); - GetStateSource(state).Message(MSG_ERROR, TEXTCOLOR_RED "%s references state %s.%d as %s state, but this state is not flagged for use as %s.\n", - obj->TypeName.GetChars(), owner->TypeName.GetChars(), int(state - owner->OwnedStates), statename.GetChars(), descript); + GetStateSource(state).Message(MSG_ERROR, TEXTCOLOR_RED "%s references state %s as %s state, but this state is not flagged for use as %s.\n", + obj->TypeName.GetChars(), FState::StaticGetStateName(state), statename.GetChars(), descript); } } if (slb->Children != nullptr)