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

Use of "relative" for book fields #42557

Closed
ghost opened this issue Jul 30, 2020 · 5 comments
Closed

Use of "relative" for book fields #42557

ghost opened this issue Jul 30, 2020 · 5 comments
Labels
[C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON stale Closed for lack of activity, but still valid. <Suggestion / Discussion> Talk it out before implementing

Comments

@ghost
Copy link

ghost commented Jul 30, 2020

Is your feature request related to a problem? Please describe.

While converting books to use abstracts to set up a looks-like hierarchy for Erk, my playtesting showed that multiple book attributes cannot be used with "relative" to change them from the parent abstract.

Describe the solution you'd like

Usage of relative for:
-"intelligence"
-"chapters"
-"time"
-"fun"
-"required_level"
-"max_level"

Describe alternatives you've considered

Declaring every field for every book that isn't exactly the same as the parent abstract.

Additional context

Books added by #37377, #40630, #40675, and #40756 are broken by the lack of this.

@anothersimulacrum anothersimulacrum added <Suggestion / Discussion> Talk it out before implementing [JSON] Changes (can be) made in JSON [C++] Changes (can be) made in C++. Previously named `Code` labels Jul 30, 2020
@Jerimee
Copy link
Contributor

Jerimee commented Jul 30, 2020

I've submitted this before as a bug at least twice, and people have said that it ought to be fixed. See #39570

That said, I think that the "cascade" of relative copy-from I attempted to implement has been decided to be more trouble than it is worth.

At the risk of quoting him out of context, KG said this about #40274

Here's the thing, this kind of hierarchy of inheritance arguably makes edits easier (I actually disagree, it makes one easy kind of edit easier, and it makes other more difficult edits harder), but it makes reading and understanding the data much more difficult. I've tried to review this PR at least five times now, and I keep getting frustrated and giving up because this is too complicated, you have to cross-reference multiple entries to find out the values of a single book, and then if you want to look at balance, you have to repeat that across multiple different books, and essentially recreate the level of redundancy you're trying to eliminate.
I need to write up some guidelines for data inheritance, but it's getting grossly overused already in areas like this and getting really unwieldy.

There are two situations where copy-from should be used in mainline.

Two things are nearly identical variants of each other.
A group of entities always (not almost always, always) shares some set of properties, then one or two levels of abstracts can set up a very shallow and narrow hierarchy.

@stale
Copy link

stale bot commented Aug 29, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

@stale stale bot added the stale Closed for lack of activity, but still valid. label Aug 29, 2020
@anothersimulacrum
Copy link
Member

#43144 Will do at least some of this.

@stale stale bot removed the stale Closed for lack of activity, but still valid. label Aug 29, 2020
@stale
Copy link

stale bot commented Sep 29, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. Please do not 'bump' or comment on this issue unless you are actively working on it. Stale issues, and stale issues that are closed are still considered.

@stale stale bot added the stale Closed for lack of activity, but still valid. label Sep 29, 2020
@stale
Copy link

stale bot commented Nov 9, 2020

This issue has been automatically closed due to lack of activity. This does not mean that we do not value the issue. Feel free to request that it be re-opened if you are going to actively work on it

@stale stale bot closed this as completed Nov 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON stale Closed for lack of activity, but still valid. <Suggestion / Discussion> Talk it out before implementing
Projects
None yet
Development

No branches or pull requests

2 participants