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

Fix for naming index bug #223 #226

Merged
merged 5 commits into from
Jun 30, 2022

Conversation

KartikShrivastava
Copy link
Contributor

A small fix for #223 which involves addition of resourceMap->AddResource(...) call after resource name creation resourceMap->CreateResourceName. This call helps to increment index counter when another resource of same type is created.

@fundies
Copy link
Contributor

fundies commented Apr 17, 2022

A small fix for #223 which involves addition of resourceMap->AddResource(...) call after resource name creation resourceMap->CreateResourceName. This call helps to increment index counter when another resource of same type is created.

Did you check if other cases like rename and delete are properly updating that map?

@KartikShrivastava
Copy link
Contributor Author

Did you check if other cases like rename and delete are properly updating that map?

Hi, I just checked for delete and indeed _resources in ResourceModelMap is not updating properly.
Here is what happens right now: After creating sprite0, sprite1 and sprite2, if I delete sprite1 and create another sprite, it is named sprite3. Which ig means that sprite1 is still there in _resources[sprite].

Rename is working properly.

I'll fix delete and make sure to test properly before creating new PRs.

@KartikShrivastava
Copy link
Contributor Author

KartikShrivastava commented Apr 17, 2022

Hi, I've tested correctness of resource name with proper index by both deleting and renaming resources for 3-4 resource types. It's working fine in both cases.

Models/ResourceModelMap.cpp Show resolved Hide resolved
MainWindow.cpp Outdated Show resolved Hide resolved
MainWindow.cpp Outdated Show resolved Hide resolved
Models/ResourceModelMap.cpp Show resolved Hide resolved
MainWindow.cpp Outdated Show resolved Hide resolved
Copy link
Member

@JoshDreamland JoshDreamland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I think we're good to go. Anything else from you, fundies?

@fundies fundies merged commit 576d6ee into enigma-dev:master Jun 30, 2022
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

Successfully merging this pull request may close these issues.

3 participants