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

Editor: Add missing items and scenery #6723

Merged
merged 1 commit into from May 27, 2022
Merged

Editor: Add missing items and scenery #6723

merged 1 commit into from May 27, 2022

Conversation

Wedge009
Copy link
Member

Resolves #6719. I didn't review #5821, I used the files that are actually present in items and scenery directories.

I'm not sure if the spelling correction for the holly-water ID will break anything (I couldn't find any references to it in cfg files). Please advise if I got anything wrong for the Lit Brazier, I'm not familiar with the animation syntax.

Back-port will need to wait for 1.16.3 string freeze to end.

@Wedge009 Wedge009 added Backport A reminder of a bugfix that was added to master that needs to be duplicated on the stable branch. Editor Map/scenario editor issues. labels May 23, 2022
@Wedge009 Wedge009 requested a review from knyghtmare May 23, 2022 00:29
@Wedge009 Wedge009 mentioned this pull request May 23, 2022
Closed
@Wedge009
Copy link
Member Author

Failing Windows checks seems to be an issue with vcpkg downloads, not this change.

@CelticMinstrel
Copy link
Member

I'm not 100% sure but I think that ID isn't actually used anywhere except in the editor internally. Maaaybe it might show up in saved scenario files? But I bet the editor uses the image path to identify the items in saved scenarios, rather than the ID.

knyghtmare
knyghtmare approved these changes May 23, 2022
View reviewed changes
Copy link
Member

@knyghtmare knyghtmare left a comment

Choose a reason for hiding this comment

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

Items -> Missing [item] for sword-short.png

can be merged as is but the missing item's part would be nice.

@CelticMinstrel
Copy link
Member

I have no idea what you're saying in that post…

@Wedge009
Copy link
Member Author

I was confused at first as well. I was just missing the entry for the Short Sword - that's been added now.

@Wedge009
Copy link
Member Author

If there are no further suggestions/concerns, I'll probably merge this soon.

@CelticMinstrel
Copy link
Member

I notice there are some groups with the same name, but I guess they're just obvious variants, right?

@Wedge009
Copy link
Member Author

You mean item1 and item2 both being called 'Item' in the user-visible string? I just followed the precedents that are already there. eg There are book variations already present that are all labelled just as 'Book'.

@Wedge009 Wedge009 merged commit 8960e40 into wesnoth:master May 27, 2022
@Wedge009 Wedge009 deleted the editor_items branch May 27, 2022 00:11
@Wedge009
Copy link
Member Author

Will back-port after 1.16.3 string freeze has ended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevecotton stevecotton stevecotton left review comments

@CelticMinstrel CelticMinstrel CelticMinstrel left review comments

@knyghtmare knyghtmare knyghtmare approved these changes

@doofus-01 doofus-01 doofus-01 left review comments

Assignees
No one assigned
Labels
Backport A reminder of a bugfix that was added to master that needs to be duplicated on the stable branch. Editor Map/scenario editor issues.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[FR] [Editor - Scenario Mode] [1.17] Add missing items and scenery
5 participants
@Wedge009 @CelticMinstrel @stevecotton @knyghtmare @doofus-01