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

Fixes deserializing rectangle on file load #748

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mradcliffe
Copy link

Summary / How this PR fixes the problem?

  • Resolves loading a saved akira file with "rectangle" type.

I found akira and started playing around with it, then ran into this bug. I figured out the file format was a zipped json file pretty quickly, opened it up, saw the inconsistency, and the found the issue. I don't really feel comfortable writing in vala, but I felt I would be comfortable enough to create a pull request for this small fix.

Steps to Test

  1. Open Akira
  2. Press the Insert button
  3. Choose Shapes
  4. Choose Rectangle
  5. Place the Rectangle
  6. Save the file
  7. Close Akira
  8. Open Akira
  9. Open the file

Result: the application does not bail with

Akira:ERROR:../src/FileFormat/JsonDeserializer.vala:113:akira_file_format_json_deserializer_deserialize_node: assertion failed: (false)

Known Issues / Things To Do

@albfan
Copy link
Collaborator

albfan commented Feb 9, 2024

To really avoid the problem, can you do another commit to change all those Magic words into constants?

@mradcliffe
Copy link
Author

can you do another commit to change all those Magic words into constants?

I reviewed to see if I could handle this. Is like the best place for this

  • enum constants within Akira.Lib.Items.Model?
  • constant in each Akira.Lib.Items.ModelTypeThing?
  • enum constants in one of the Managers?

Regardless it seems like model name_id method should return that constant as well.

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.

2 participants