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.

Sign up for GitHub

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

Jump to bottom

Refactored tilespec.cpp code. Part of the progress on #2232 #2241

Merged
merged 5 commits into from
Mar 28, 2024
Merged

Refactored tilespec.cpp code. Part of the progress on #2232 #2241

merged 5 commits into from
Mar 28, 2024

Conversation

hugomflavio
Copy link
Contributor

No description provided.

@hugomflavio
Copy link
Contributor Author

@jwrober we need you to review this one, please :)

Not sure what the clang check is complaining about, I ran clang before committing 🤔

@jwrober
Copy link
Collaborator

jwrober commented Mar 27, 2024

Me either. I can take a look this afternoon

@lmoureaux
Copy link
Contributor

Not sure what the clang check is complaining about, I ran clang before committing 🤔

Looks like the classic issue of a clang-format version mismatch. We use v11 which is getting old

@lmoureaux lmoureaux requested a review from jwrober March 27, 2024 20:16
@jwrober jwrober mentioned this pull request Mar 28, 2024
Closed
@jwrober
Copy link
Collaborator

jwrober commented Mar 28, 2024

I'm getting warnings

[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching unit.action_decision_want
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching city.size_000
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching path.turns_000 or city.size_000
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching path.steps_000, path.turns_000, or city.size_000
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching path.exhausted_mp_000, path.turns_000, or city.size_000
[info] freeciv21-client ( /tilespec.cpp:1004) - Loading tileset "amplio2".
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching unit.action_decision_want
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.road_c_ne
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.road_c_se
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.road_c_sw
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.road_c_nw
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.rail_c_ne
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.rail_c_se
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.rail_c_sw
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.rail_c_nw
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.maglev_c_ne
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.maglev_c_se
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.maglev_c_sw
[warning] freeciv21-client ( /tilespec.cpp:412) - Could not find optional sprite matching road.maglev_c_nw

Is this expected?

@lmoureaux
Copy link
Contributor

No. Did you check the tileset debugger? It might also have many new messages...

@hugomflavio
Copy link
Contributor Author

hugomflavio commented Mar 28, 2024 via email

@jwrober
Copy link
Collaborator

jwrober commented Mar 28, 2024

I'm testing on Amplio2 (squares)

@hugomflavio
Copy link
Contributor Author

hugomflavio commented Mar 28, 2024 •
edited
Loading

Do you get those warnings for hexemplio?
And does Amplio2 have those sprites reported as missing

@hugomflavio
Copy link
Contributor Author

Could not find optional sprite matching city.size_000

These look like real bugs though. Pretty sure that should end in 100?

@lmoureaux
Copy link
Contributor

Could not find optional sprite matching city.size_000

These look like real bugs though. Pretty sure that should end in 100?

I bet there is no 000 sprite because it would only be used starting from 1000, which isn't supported

@jwrober
Copy link
Collaborator

jwrober commented Mar 28, 2024

Aplio2 Debugger
image

@jwrober
Copy link
Collaborator

jwrober commented Mar 28, 2024

Hexemplio Debugger
image

@hugomflavio
Copy link
Contributor Author

@lmoureaux do you recall if there were warnings for optional sprites before?

@lmoureaux
Copy link
Contributor

@lmoureaux do you recall if there were warnings for optional sprites before?

There weren't, we may want to downgrade to info

@hugomflavio
Copy link
Contributor Author

Make it a note? Sounds good by me

@jwrober jwrober merged commit 8824360 into longturn:master Mar 28, 2024
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwrober jwrober jwrober approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hugomflavio @jwrober @lmoureaux