Announcement

Collapse
No announcement yet.

Storing objects in TMap..what am I doing wrong here?

Collapse
X
 
  • Filter
  • Time
  • Show
Clear All
new posts

    Storing objects in TMap..what am I doing wrong here?

    Hey all,

    I am more used to working in languages where I don't have to worry about memory management and pointers and what not, so I am sure this is just me doing something stupid, but I am not sure what.

    In a setup routine I am creating instances of a UObject based class, and sticking a pointer to each into a TMap. Then later on I am iterating through the TMap to do things with them. However when I get the instance out of the TMap later, the data inside it is not correct!

    Here is my setup
    Code:
    void AHexGridActor::CreateGrid()
    {
    	if (!DirtyCreate)
    		return;
    
    	UE_LOG(LogTemp, Display, TEXT("CreateGrid"));
    	DirtyCreate = false;
    	Tiles.Reserve(Radius * Radius * Radius);
    	int32 tileCount = 0;
    	for (int32 i = -Radius; i <= Radius; i++)
    	{
    		for (int32 j = -Radius; j <= Radius; j++)
    		{
    			FString tileName = FString::Printf(TEXT("HexTile_%d"), tileCount++);
    			UHexTile* newTile = NewObject<UHexTile>(this, UHexTile::StaticClass(), *tileName);
    			tileCount++;
    			if (newTile == nullptr)
    			{
    				UE_LOG(LogTemp, Error, TEXT("Failed to create HexTile object"))
    			}
    			else
    			{
    				FString tileAddress = FString::Printf(TEXT("%d,%d"), i, j);
    				newTile->Position = FHexCoordinate{ 0, 0 };
    				newTile->Transform = FTransform(FVector(100,10,50));
    				newTile->Transform.NormalizeRotation();
    				newTile->Dirty = true;
    				newTile->Valid = true;
    				newTile->OnCreateTile();
    				UE_LOG(LogTemp, Display, TEXT("Tile's transform: %s"), *newTile->Transform.ToString());
    				if (newTile->Valid)
    				{
    					UE_LOG(LogTemp, Warning, TEXT("Skipping tile, as is marked invalid"));
    					Tiles.Add(tileAddress, newTile);
    				}
    			}
    		}
    	}
    	DirtyDraw = true;
    }
    And later usage
    Code:
    void AHexGridActor::DrawGrid()
    {
    	for (auto& Entry : Tiles)
    	{
    		UHexTile* tile = Entry.Value;
    		/*tile->OnDrawTile();*/
    		UStaticMesh* tileMesh = tile->Mesh;
    		if (tileMesh != nullptr)
    		{
    			if (TileMeshInstances.Contains(tileMesh))
    			{
    				UHierarchicalInstancedStaticMeshComponent* hism = TileMeshInstances[tileMesh];
    				hism->AddInstance(tile->Transform);
    			}
    			else
    			{
    				int32 hismCount = TileMeshInstances.Num();
    				FString hismName = FString::Printf(TEXT("TileMeshInstance_%d"), hismCount);
    				UHierarchicalInstancedStaticMeshComponent* hism = NewObject<UHierarchicalInstancedStaticMeshComponent>(this, UHierarchicalInstancedStaticMeshComponent::StaticClass(), *hismName);
    				hism->RegisterComponent();
    				hism->AttachTo(RootComponent);
    				UE_LOG(LogTemp, Display, TEXT("Using transform for instance: %s"), *tile->Transform.ToString());
    				hism->AddInstance(tile->Transform);
    				TileMeshInstances.Add(tileMesh, hism);
    			}
    		}
    	}
    }

    #2
    Is your TMap declared as a UPROPERTY to prevent the data inside from being garbage collected? UObjects are automatically memory managed by reference counting, but only UPROPERTIES contribute to that count.
    Journeyman's Minimap - Available on Marketplace - Forum topic - Video

    Comment


      #3
      TMap can't be a UPROPERTY (Which is a real shame), but the documentation on it says that the TMap maintains a strong reference to anything stored in it.

      Comment


        #4
        I just checked and TMap UPROPERTIES are now possible. This must be a recent change. I checked with the latest build from source, maybe this isn't in the 4.9 release version yet.

        Anyway, could you try keeping copies of those UHexTiles in a TArray and see if that makes a difference?
        Journeyman's Minimap - Available on Marketplace - Forum topic - Video

        Comment


          #5
          Originally posted by John Vanderbeck View Post
          TMap can't be a UPROPERTY (Which is a real shame), but the documentation on it says that the TMap maintains a strong reference to anything stored in it.
          You can certainly have:
          Code:
          	UPROPERTY()
          	TMap<Key*, Value*> MyMap;
          All pointers either as keys or values will not get garbage collected until removed from it.
          • Follow me on twitter
          • Visit our website traverse.world
          • Checkout our game's forum thread

          Comment


            #6
            I swear I tried to make it a UPROPERTY and got an error. I guess I need to look again.

            Comment


              #7
              Originally posted by John Vanderbeck View Post
              I swear I tried to make it a UPROPERTY and got an error. I guess I need to look again.
              You can't have UPROPERTY(BlueprintReadOnly) or UPROPERTY(BlueprintReadWrite) TMaps... (and that IS a shame but you can imagine how it would complicate UI for blueprints with unpredictable nesting where key can be anything even big struct, and so can value)
              • Follow me on twitter
              • Visit our website traverse.world
              • Checkout our game's forum thread

              Comment


                #8
                @xulture: Been meaning to look into this, but I don't suppose you have any idea how TMap/TSet behave with regards to UObject keys and destruction of stored keys? As far as I know, generally maps and sets are implemented with the restriction that their keys must be immutable. I'm wondering what happens if you have a UPROPERTY TMap/TSet with UObject pointer keys (without duplicate keys allowed) and one of those keys gets destroyed. Is it designed to handle this case?

                I know from looking at pre-4.8 engine code that there are a couple of places where a TMap was used with a TWeakObjectPtr key, which gives rise to the same potential situation. But perhaps this should only be done when you know for sure the lifetime of the keys will encompass that of the map?

                Comment


                  #9
                  When you use pointers as keys, they are really used just as integers. If you manually destroy UObject that it points to, value of the key remains the same; and you could cause crash when attempting to use the pointer.. TMaps integrity, however, remains intact.

                  Equality of two pointers is true only if they point to the same location,

                  If you let UE do garbage collection and destruction (by marking TMap as UPROPERTY()), objects will be cleaned up only after you remove the keys.

                  One of our uses for TMaps is exactly to ensure cleanup happens after objects are removed from the TMap.
                  • Follow me on twitter
                  • Visit our website traverse.world
                  • Checkout our game's forum thread

                  Comment


                    #10
                    Hmm, my understanding was that in the case of manual destruction, UPROPERTY pointers would get nulled out (just as TWeakObjectPtrs would become stale). I think I read this in the context of AActors anyway, where it is normal that they can be destroyed even when strong references are still held. It was this scenario that I was wondering about, since nulling out pointers which are keys in a map would lead to non-uniqueness.

                    Anyway it's a corner case, I know. I'll get around to doing a test at some point!

                    Comment


                      #11
                      Adding just a blank UPROPERTY() was indeed my issue. Thanks!

                      Comment


                        #12
                        There are a few reasons not to use TMap as your exclusive storage for multiple objects, one is it is a pain to not have it garbage collect, and two is it does not replicate for multiplayer games (at least not the last time I checked). To properly utilize TMap do not use it to hold the actual objects, use it to hold the index into an array which is compatible with UPROPRETY and will not be garbage collected and replicates easily.

                        So you have the following configuration:

                        Code:
                        TMap<SomeKeyType, int32> objectIndex;
                        
                        TArray<UMYObjects> objects;
                        Each time you insert into the array add an entry into the ObjectIndex to keep it updated. Each time you remove from the array, clean up the index. Now any Get functions you need that pass in a Key value you just use:

                        Code:
                        return objects[objectIndex.Find(identifier)]
                        of course you probably want to wrap this up in some fancy validation statements to ensure that the key exists and that the returned index from Find is within the range and bounds of the array... but you should get the jest. I actually simplied my life greatly by creating an interface for managing array index maps.

                        For replicating, the Array is replicated and an OnRep function is called to build the indexes on the local client (if such a thing is needed). This way any time the number of objects in the array changes the indexes are updated. Your standard add and remove object functions will keep the server indexes up to date but the client is a different story, hense the on rep function. Also consider the client may not need to know any of these indexes or the client may be the only thing that needs to know these indexes.
                        Last edited by ArcainOne; 09-09-2015, 04:09 PM.

                        Comment


                          #13
                          @ArcainOne: As of 4.8 this is no longer the case, TMap is now a supported property type with garbage collection and replication. As far as I'm aware, the only limitation is that it is not yet integrated into the editor/BP in a convenient way.

                          Comment


                            #14
                            If this is the case, it'd be great if https://wiki.unrealengine.com/Garbag...ction_Overview (and some of the other garbage collection articles!) were updated to reflect TMap's new powers

                            Comment


                              #15
                              TMap's still can't be replicated and nor can TSets, they are just reflected types.

                              Also that's a hell of a bump. Better next time to make requests inside the documentation forum.

                              Comment

                              Working...
                              X