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 icon loading #3450

Merged
merged 25 commits into from
Oct 3, 2021
Merged

Conversation

Garanas
Copy link
Member

@Garanas Garanas commented Sep 26, 2021

This branch will try to fix the icon loading procedure that was added during the 3721 patch. It is very broken from many perspectives, including:

  • It loads in textures regardless of whether the mod is active.
  • It loads in all ui textures, regardless of whether they're icons.

These two combined can result in strange behavior, such as inactive mods interfering with the base game. An example as reported by FtGr is the following:
image
image

These are probably defined in some mod on his system. They override the default icons on the UI.

@Garanas
Copy link
Member Author

Garanas commented Sep 26, 2021

The pipeline works in terms of receiving the data at the right moment. But - no icon is rendered. Thoughts on what could be going wrong? I suspect it is the path.

You can use this mod to see the logs happening, make sure it is activated:
Penguins Icon Mod.zip

image

image

An example of how it works right now with the same unit, which is a path that starts at the folder in question (/textures/ui/something/something/strategicicons).

Downside of current approach: if the sim would use this value in the blueprint for some reason then it would desync on the spot. I do not expect this to be the case however. I have not yet checked if it desyncs the game when multiply people have different ui mods that change the icons.

@Garanas
Copy link
Member Author

Garanas commented Sep 26, 2021

I think I got it working reliably now, here you have a mod to test with:

I can't check if it causes a desync in multiplayer - when testing locally both instances try to read the same prefs file and it causes one to lock out the other - so the other crashes.

I need someone else to help test that.

@Garanas
Copy link
Member Author

Garanas commented Sep 26, 2021

Keyser and I tested this PR tonight - it works! 🪂

@Garanas
Copy link
Member Author

Garanas commented Sep 27, 2021

And a few examples on how to work with this new approach to loading icons:

With thanks to Balthazar for the discussions and the alternative icon set for ASI.

@Garanas Garanas linked an issue Sep 27, 2021 that may be closed by this pull request
@Garanas Garanas marked this pull request as ready for review September 27, 2021 19:53
@Garanas Garanas requested a review from Crotalus September 27, 2021 19:53
@Garanas
Copy link
Member Author

Garanas commented Sep 27, 2021

A few notes on this PR:

  • Prefetching is disabled again. We can not send data to blueprints.lua reliably when prefetching is enabled.
  • This PR may cause the game to crash when you have multiple instances of the game on the same computer in the same lobby, all starting at once. As the game writes the preference file on launch, blocking it for other instances.

@Garanas Garanas added this to the 3724 milestone Sep 29, 2021
@Dragun123
Copy link
Contributor

Do I need change my icons file structure now?

@Garanas
Copy link
Member Author

Garanas commented Sep 29, 2021

Not sure - if it is for a sim mod then I don't think so. If it is for a UI mod then yes, 100% certain.

In particular, a lua parsing error does not appear to stop
executing and therefore it not caught by the pcall we used.

Instead, we're now checking if either values that we expect are
set and if none of them are then we expect something to be at
odds.
changelog-3724.md Outdated Show resolved Hide resolved
@Garanas Garanas merged commit cca5e78 into FAForever:deploy/fafdevelop Oct 3, 2021
@Garanas Garanas added the feature: strategic icons related to strategic icons label Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: strategic icons related to strategic icons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

icon mod support
3 participants