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

PAWStockGroups - Generators #235

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

Conversation

yalov
Copy link

@yalov yalov commented Jun 21, 2024

Add Generator PAW Group,
a separate group per every generator

1 2

Without groups, there is no information about which generator and action the fields are related to.

@JonnyOThan
Copy link
Contributor

let's check core heat and thermal efficiency too
image

@yalov
Copy link
Author

yalov commented Aug 26, 2024

ModuleResourceConverter that have coretemp and termal efficiency is not the ModuleGenerator.
ModuleResourceConverter does not have the same problem:

3

@gotmachine
Copy link
Contributor

I would recommend adding a check to only put generators in groups if there are multiple generators on the part.

@JonnyOThan
Copy link
Contributor

Yeah agreed; and sorry I guess that should be filed as a new feature request rather than a comment on a PR - I still need to go over this in detail but it looks good.

@yalov
Copy link
Author

yalov commented Aug 30, 2024

I would recommend adding a check to only put generators in groups if there are multiple generators on the part.

@gotmachine , why? it groups action and fields together, and adds resources abbr

@gotmachine
Copy link
Contributor

gotmachine commented Aug 31, 2024

why? it groups action and fields together, and adds resources abbr

From the feedback I've got, people tend to dislike having to go through the extra step of having to expand groups, especially when the PAW doesn't have much stuff. In general, there is a lot of negative feedback on the whole "extra groups" patch. At least I would set those generator groups as always expanded by default, set the last arg to false instead of the conditional on module count :

BasePAWGroup pawGroup = new BasePAWGroup(name, generatorGroupTitle + abbrs, __instance.part.Modules.Count > 3);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants