Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reimporting animations sets references to sprites to 0, breaks reference when moving computers #58

Closed
GieziJo opened this issue Feb 1, 2023 · 13 comments

Comments

@GieziJo
Copy link
Collaborator

GieziJo commented Feb 1, 2023

As referenced in this post in unity, when reimporting a file, the metadata reference gets broken, and sets the Ids to 0.

Example:
image

They should all have IDs.

Unity version 2022.1.20f

@tuntorius
Copy link

tuntorius commented Mar 15, 2023

It gets broken on the same computer as well. If you add 2 or more new sprites to a sheet at the same time, they all get ids of 0 and can't be distinguished from one another. This is because unity made TextureImporter.spriteSheet property obsolete. Not deprecated, but obsolete - it stopped working at all for Unity 2021.2+. For that version upwards, you need to use UnityEditor.U2D.Sprites.ISpriteEditorDataProvider interface instead:
https://docs.unity3d.com/2022.2/Documentation/ScriptReference/TextureImporter-spritesheet.html

@GieziJo
Copy link
Collaborator Author

GieziJo commented Mar 16, 2023

Thanks for that.

Agreed, I also had issues with the same computer as well.

I'm not sure it;s the root of the problem, it's only obsolete for versions > 2022.1, and I'm using that one, but still getting the error.

Maybe it's not obsolete but not actually working correctly?

I also saw that if you re-import the file ids get completely screwed up.

My local fix, which kinda worked, but means re-linking all images in the animation is to delete file table id in the meta file, when it gets regenerated it then works.

Honestly I'm a bit lost with this one, not sure how to fix it, would be super happy to get help.

@tuntorius
Copy link

The page I linked clearly says it's obsolete. It also says "Support for accessing sprite meta data through spritesheet has been removed."
It's obsolete since 2021.2.0a7, not 2022. The older docs are not updated.

This issue shows which is the first version with this change:
https://issuetracker.unity3d.com/issues/identifier-uniqueness-violation-warning-go-loses-sprite-when-using-textureimporter-dot-spritesheet-to-split-an-image-into-sprites
It has links explaining the new APIs.

I can only give general pointers on how to fix it. The spritesheet property must be replaced with the new API. It looks like it's used here:

importer.spritesheet = animationSheet.GetSpriteSheet(

Here are some examples of how to use the new API:
https://docs.unity3d.com/Packages/com.unity.2d.sprite@1.0/manual/DataProvider.html

First, you need to obtain ISpriteEditorDataProvider instance, which the first example shows how.
Then, the spritesheet metadata has to be updated. The example "How to change Sprite data" shows that.
This line:
dataProvider.SetSpriteRects(spriteRects);
is the equivalent of the old and broken:
importer.spritesheet = ...
They both receive an array of sprite rects that describe the spritesheet. However, the old one uses UnityEditor.SpriteMetadata, the new one UnityEditor.SpriteRect. They are different classes, so not interchangeable, but somewhat similar. They both have pivot, border, rect.
However, the new one has spriteID!!! This is the ID that becomes 0.
It's missing from the old one.

Here it becomes complicated and I can't help anymore. Now it's the responsibility of the plugin to manipulate that ID. For existing frames, it must retrieve the ID from the spritesheet and keep it the same. For new frames it must generate a new ID using GUID.Generate();
This means the code behind animationSheet.GetSpriteSheet() must be updated to use the new SpriteRect classes and respect the IDs. It looks like a lot of work.

I'm not using this plugin. I use another, which is just a sprite packer. It had the exact same problem. The change there was easier and I fixed it. It took me several days though.

@GieziJo
Copy link
Collaborator Author

GieziJo commented Mar 16, 2023

The older docs are not updated.

Ah okay, that's why I got confused, thanks for the clarification.

Thanks a lot for the details on how you made it all work for this other package.

I am not the main developer of the tool, I just helped lately with a couple of new features, so I'm not sure I would be able to do these changes, but I can try.

I don't know if @talecrafter is still interested in keeping this tool updated? I know a lot of people are actively using it and would probably be glad if this gets fixed, but I understand if this is not a priority anymore.

Thanks a lot for your inputs!

Repository owner deleted a comment from toolcrafter Mar 17, 2023
@talecrafter
Copy link
Owner

I will take a look at the problem.

Sorry for not maintaining this tool lately. I was really busy and overwhelmed on my own project, and still am.

I'm using 2021.3 currently and the sprite import seems to work fine still without compilation error. (Not 100% sure I'm still at the exact same code as this Github version.) But occasionally I get a warning about invalid IDs on the sprites, so I guess that's related.

@talecrafter
Copy link
Owner

Does anyone have an idea why the UnityEditor.U2D.Sprites namespace is not available while the com.talecrafter.animationimporter Assembly definition exists? I'm using Unity 2021.3 and once I delete that assembly definition, the new API becomes available.

@tuntorius
Copy link

You need to add a reference to Unity.2D.Sprite.Editor to your assembly definition.

@talecrafter
Copy link
Owner

talecrafter commented Mar 17, 2023

Thank you, @tuntorius. That was indeed the issue.

@GieziJo:
I implemented a simple version of reusing the Sprite IDs on a new beta branch.
For the Unity package, you would use this url:
https://github.com/talecrafter/AnimationImporter.git?path=/Assets#beta
Can you check if this version fixes your issues?

@GieziJo
Copy link
Collaborator Author

GieziJo commented Mar 17, 2023

@talecrafter Thanks a lot!

Will check this this weekend and let you know.

@GieziJo
Copy link
Collaborator Author

GieziJo commented Mar 19, 2023

I think this worked!

I had an issue with the importer, where if animations have been imported incorrectly, the spriteID is set to 00000000000000000800000000000000 and is the same for all sprites.

I added a catch and regenerate the ID if it is detected (here):

try
{
	dataProvider.SetSpriteRects(newSpriteRects);
}
catch (Exception _)
{
	for (int i = 0; i < newSpriteRects.Length; i++)
	{
		if(newSpriteRects[i].spriteID == new GUID("00000000000000000800000000000000"))
			newSpriteRects[i].spriteID = GUID.Generate();
	}
	dataProvider.SetSpriteRects(newSpriteRects);
}

I think this is not necessary if a sprite had not been imported with a version where the importer is obsolete, but it was in my case.

Are we happy with including this or is there a better option for that?

Thanks a lot for the updated package!

@talecrafter
Copy link
Owner

I saw this specific fault ID in a handful of my assets, too.
Refactored the check for that ID to be part of IsValidGUID in ImportedAnimationSheet. As far as I can see there's no need to retry changing the GUIDs on Exception as the IDs in newSpriteRects should be fine and if there's an error, it's due to other problems.

@talecrafter
Copy link
Owner

I merged this on the main branch and deleted the beta branch (for now).

https://github.com/talecrafter/AnimationImporter/releases/tag/1.5

Closing this now.

@GieziJo
Copy link
Collaborator Author

GieziJo commented Apr 6, 2023

That is a much cleaner solution, awesome!

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants