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

Sync Dialog_BillConfig target item style change #402

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

Conversation

SokyranTheDragon
Copy link
Member

The feature is only available with Ideology DLC active while the ideo system is disabled.

All of the prefixed methods could be turned into sync methods/delegates, but it would require making a sync worker for Dialog_BillConfig or making an instance/field transformer for it, so it seemed using sync fields would end up simpler.

The feature is only available with Ideology DLC active while the ideo system is disabled.

All of the prefixed methods could be turned into sync methods/delegates, but it would require making a sync worker for Dialog_BillConfig or making an instance/field transformer for it, so it seemed using sync fields would end up simpler.
@Zetrith
Copy link
Member

Zetrith commented Nov 20, 2023

Neither this nor SyncMethods work well here I think. I don't like that this solution uses compiler-generated names.

The best solution would be to have some sort of a general system where you can make lambdas created in a given method (or just FloatMenu lambdas?) inherit/remember the SyncField context.

@SokyranTheDragon
Copy link
Member Author

Yeah, I agree. I was rather unhappy that I had to end up using compiler-generated names, but couldn't really come up with an alternative that felt as good.

If we want to avoid using compiler-generated names while also not having to create any new systems, then I believe using SyncMethods/Delegates could work here if we made a SyncWorker for Dialog_BillConfig. The only reason I used those fields by name was to access Dialog_BillConfig.bill which, if we made a SyncWorker, would not longer be needed.

And as for creating some system to handle situations like those... Well, I tend to over-complicate stuff like that, so I'm not sure if I should attempt to take care of it.

@SokyranTheDragon
Copy link
Member Author

Actually, I have an idea on how to (potentially) handle this... @Zetrith what do you think about making an overload for SyncDelegate.TransformField, but instead of working on specific fields by name - they'd apply to fields of specific type?

@SokyranTheDragon SokyranTheDragon added the ideology Fix or bugs relating to Ideology (Not 1.3) label Dec 4, 2023
@Zetrith
Copy link
Member

Zetrith commented Apr 16, 2024

I'll try implementing the general solution I mentioned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ideology Fix or bugs relating to Ideology (Not 1.3)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants