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

Request: A mod to disable the freezing changes. #25542

Closed
WizardOfOoo opened this Issue Sep 11, 2018 · 17 comments

Comments

Projects
None yet
10 participants
@WizardOfOoo
Contributor

WizardOfOoo commented Sep 11, 2018

Something to disable the changes to temperature made after build 7700

@Regularitee

This comment has been minimized.

Show comment
Hide comment
@Regularitee
Contributor

Regularitee commented Sep 11, 2018

@lispcoc

This comment has been minimized.

Show comment
Hide comment
@lispcoc

lispcoc Sep 11, 2018

Contributor

Insert NO_FREEZE flag to every items after #25509 merged.

Contributor

lispcoc commented Sep 11, 2018

Insert NO_FREEZE flag to every items after #25509 merged.

@WizardOfOoo

This comment has been minimized.

Show comment
Hide comment
@WizardOfOoo

WizardOfOoo Sep 11, 2018

Contributor

Kevin says no!

I don't understand why.
We have mods to turn all sorts of things off. What is so special about this mechanic?

Contributor

WizardOfOoo commented Sep 11, 2018

Kevin says no!

I don't understand why.
We have mods to turn all sorts of things off. What is so special about this mechanic?

@CoroNaut

This comment has been minimized.

Show comment
Hide comment
@CoroNaut

CoroNaut Sep 11, 2018

Kevin has full authority over stuff (from my suggestion to add an atomic item):

If someone wants to add tentacle monsters I should let them? No, turning
down suggestions that don't fit in the game is one of my responsibilities.
I did forget to mention that someone can feel free to add an "atompunk" or
whatever mod that adds more extensive uses of plutonium if they like.

He has said in multiple other suggestions that creating a mod of it was ok. I don't see why he turned down this one (about a no freezing mod) in particular.

CoroNaut commented Sep 11, 2018

Kevin has full authority over stuff (from my suggestion to add an atomic item):

If someone wants to add tentacle monsters I should let them? No, turning
down suggestions that don't fit in the game is one of my responsibilities.
I did forget to mention that someone can feel free to add an "atompunk" or
whatever mod that adds more extensive uses of plutonium if they like.

He has said in multiple other suggestions that creating a mod of it was ok. I don't see why he turned down this one (about a no freezing mod) in particular.

@cake-pie

This comment has been minimized.

Show comment
Hide comment
@cake-pie

cake-pie Sep 11, 2018

Contributor

Insert NO_FREEZE flag to every items after #25509 merged.

That doesn't do what you think it does. NO_FREEZE is used for foods that would suffer detrimental effects when frozen, i.e. turn MUSHY when thawed. #25509 will rename NO_FREEZE to FREEZERBURN which will hopefully be less confusing about the purpose of the flag.

Contributor

cake-pie commented Sep 11, 2018

Insert NO_FREEZE flag to every items after #25509 merged.

That doesn't do what you think it does. NO_FREEZE is used for foods that would suffer detrimental effects when frozen, i.e. turn MUSHY when thawed. #25509 will rename NO_FREEZE to FREEZERBURN which will hopefully be less confusing about the purpose of the flag.

@lispcoc

This comment has been minimized.

Show comment
Hide comment
@lispcoc

lispcoc Sep 11, 2018

Contributor

oops, I didn't know NO_FREEZE flag is replaced with freezing points idea.

Contributor

lispcoc commented Sep 11, 2018

oops, I didn't know NO_FREEZE flag is replaced with freezing points idea.

@Kelenius

This comment has been minimized.

Show comment
Hide comment
@Kelenius

Kelenius Sep 11, 2018

Contributor

Set freezing point to -274 on everything.

Contributor

Kelenius commented Sep 11, 2018

Set freezing point to -274 on everything.

@cake-pie

This comment has been minimized.

Show comment
Hide comment
@cake-pie

cake-pie Sep 11, 2018

Contributor

Overriding freezing point of foods in json would need updated when items added/removed; implementing an EXTERNAL_OPTION in code (like Filthy Clothing / Simplified Nutrition) doesn't require such maintenance.

Contributor

cake-pie commented Sep 11, 2018

Overriding freezing point of foods in json would need updated when items added/removed; implementing an EXTERNAL_OPTION in code (like Filthy Clothing / Simplified Nutrition) doesn't require such maintenance.

@WizardOfOoo

This comment has been minimized.

Show comment
Hide comment
@WizardOfOoo

WizardOfOoo Sep 11, 2018

Contributor

What about a bit of code that always returns -274 when the game asks for freezing temperature?

Contributor

WizardOfOoo commented Sep 11, 2018

What about a bit of code that always returns -274 when the game asks for freezing temperature?

@alanbrady

This comment has been minimized.

Show comment
Hide comment
@alanbrady

alanbrady Sep 11, 2018

Contributor

I don't want to speak for @kevingranade, but I believe his objection to the previous attempt was because it was too intrusive and fragile the way it was done. After #25509 is complete, implementing a mod in a way that isn't super fragile or intrusive should be totally possible. Note, I am not volunteering to do that mod per se, just clarifying the situation.

Contributor

alanbrady commented Sep 11, 2018

I don't want to speak for @kevingranade, but I believe his objection to the previous attempt was because it was too intrusive and fragile the way it was done. After #25509 is complete, implementing a mod in a way that isn't super fragile or intrusive should be totally possible. Note, I am not volunteering to do that mod per se, just clarifying the situation.

@Regularitee

This comment has been minimized.

Show comment
Hide comment
@Regularitee

Regularitee Sep 12, 2018

Contributor

"I'm not interested in confusing the issue further by disabling the [freezing] feature."

Given his last issue closed with indication of what you are claiming his reasons are, you are indeed speaking for him. No offense.

Kevin has the bad habit of shutting down issues with little more than a very vague statement (see above), leaving many confused as to what rule or policy they ran afoul of. Which in turn leads to rehashing of "closed" issues like this (since no one is entirely certain what went wrong with the previous attempt, and may inadvertently repeat the same mistake). Or just cultivating hostility to moderators by creating the appearance of arbitrary decision-making.

It would help either speed along the process of creating a mod, or shutting down a doomed endeavor early, if there would be some clarification of what was wrong with the previous attempt.

Contributor

Regularitee commented Sep 12, 2018

"I'm not interested in confusing the issue further by disabling the [freezing] feature."

Given his last issue closed with indication of what you are claiming his reasons are, you are indeed speaking for him. No offense.

Kevin has the bad habit of shutting down issues with little more than a very vague statement (see above), leaving many confused as to what rule or policy they ran afoul of. Which in turn leads to rehashing of "closed" issues like this (since no one is entirely certain what went wrong with the previous attempt, and may inadvertently repeat the same mistake). Or just cultivating hostility to moderators by creating the appearance of arbitrary decision-making.

It would help either speed along the process of creating a mod, or shutting down a doomed endeavor early, if there would be some clarification of what was wrong with the previous attempt.

@cake-pie

This comment has been minimized.

Show comment
Hide comment
@cake-pie

cake-pie Sep 12, 2018

Contributor

I believe his objection to the previous attempt was because it was too intrusive and fragile the way it was done. After #25509 is complete, implementing a mod in a way that isn't super fragile or intrusive should be totally possible.

You raised concerns #25385 (comment) in my PR:

The apply_in_fridge gets called every item to be processed in a vehicle cargo space. I would recommend not using get_option here because it's slow and you don't need to get_option every call. Please cache this value in some way.

Additionally, this only handles things in a vehicle and doesn't prevent things outside from freezing AFAICT Nevermind looking further item::process_food calls apply_in_fridge which is weird because item::process_food does a bunch of temperature calculations in addition to the ones done in apply_in_fridge. I'm pretty sure this also means apply_in_fridge gets called twice for any given item in a vehicle.

This seems like a root cause of some of the finicky temperature calculations, especially the case where an item is being both heated up by the environment and cooled by the fridge, which is wrong to begin with IMHO.

The first para raises concerns that what I did would be costly/slow. I took a stab at addressing that in f0c8e20 but since my PR already got shut down I saw no point trying to contribute those changes here.

The rest of your post is comment on the pre-existing temperature handling code, which has nothing to do with me or my PR; #25509 is you addressing those misgivings.

The crux of my code is adding a check in the one and only place where item freezing occurs in the code, to prevent freezing if the user has disabled it through the mod. It is a one-line change in exactly the place it needs to be.

After #25509 lands I would simply move it here instead, kicking in at precisely the same place where a modded freezing temp would prevent the item from freezing.

Would you care to elaborate on why you think this is "too intrusive and fragile", since this is your opinion, not something Kevin said.

Contributor

cake-pie commented Sep 12, 2018

I believe his objection to the previous attempt was because it was too intrusive and fragile the way it was done. After #25509 is complete, implementing a mod in a way that isn't super fragile or intrusive should be totally possible.

You raised concerns #25385 (comment) in my PR:

The apply_in_fridge gets called every item to be processed in a vehicle cargo space. I would recommend not using get_option here because it's slow and you don't need to get_option every call. Please cache this value in some way.

Additionally, this only handles things in a vehicle and doesn't prevent things outside from freezing AFAICT Nevermind looking further item::process_food calls apply_in_fridge which is weird because item::process_food does a bunch of temperature calculations in addition to the ones done in apply_in_fridge. I'm pretty sure this also means apply_in_fridge gets called twice for any given item in a vehicle.

This seems like a root cause of some of the finicky temperature calculations, especially the case where an item is being both heated up by the environment and cooled by the fridge, which is wrong to begin with IMHO.

The first para raises concerns that what I did would be costly/slow. I took a stab at addressing that in f0c8e20 but since my PR already got shut down I saw no point trying to contribute those changes here.

The rest of your post is comment on the pre-existing temperature handling code, which has nothing to do with me or my PR; #25509 is you addressing those misgivings.

The crux of my code is adding a check in the one and only place where item freezing occurs in the code, to prevent freezing if the user has disabled it through the mod. It is a one-line change in exactly the place it needs to be.

After #25509 lands I would simply move it here instead, kicking in at precisely the same place where a modded freezing temp would prevent the item from freezing.

Would you care to elaborate on why you think this is "too intrusive and fragile", since this is your opinion, not something Kevin said.

@alanbrady

This comment has been minimized.

Show comment
Hide comment
@alanbrady

alanbrady Sep 12, 2018

Contributor

Would you care to elaborate on why you think this is "too intrusive and fragile", since this is your opinion, not something Kevin said.

screenshot from 2018-09-11 21-27-28

Contributor

alanbrady commented Sep 12, 2018

Would you care to elaborate on why you think this is "too intrusive and fragile", since this is your opinion, not something Kevin said.

screenshot from 2018-09-11 21-27-28

@tyrael93

This comment has been minimized.

Show comment
Hide comment
@tyrael93

tyrael93 Sep 12, 2018

Couldn't you just add that snippet of code in the no freezing mod as an external LUA file and make the mod run just fine like the no bionic installation mod did?

Kevin says no, well tough shit then take matters in your own hands.

tyrael93 commented Sep 12, 2018

Couldn't you just add that snippet of code in the no freezing mod as an external LUA file and make the mod run just fine like the no bionic installation mod did?

Kevin says no, well tough shit then take matters in your own hands.

@cake-pie

This comment has been minimized.

Show comment
Hide comment
@cake-pie

cake-pie Sep 12, 2018

Contributor

I don't buy the reasoning behind the decision to block any further mods that disable core features.

The recent change in default season length is accompanied by the announcement

[this] announces the intent that choosing non-standard timespans can and will cause issues

Well, equally, when a user applies mods to their game they should also beware issues, e.g. wonky balance, unexpected interactions or even outright breakages. This is true of any game, not just CDDA.

Why the difference in attitude?

Precluding mods from making even tiny code changes -- i.e. insert a check for some user-configurable option -- severely limits the range of what is possible.

In this case (no freezing), what could be done with very few lines of code would require a much larger json mod, and even that would not be possible if not for freezing point json setting to be added soon.

If we consider, say, Simplified Nutrition or Filthy Clothing, it's not clear those mods can even exist if they're not allowed to touch C++ code. For one, filthy clothing mechanics doesn't translate to something that can be straightforward jsonized and modded with purely json changes.

The inflexible philosophy of "core features can no longer be made optional" means that when Hypothetical Future Controversial Core Feature lands, it may not be possible to disable it within these limitations. The only recourse for people who don't want to play such a feature would be a hostile fork; I'd hate for that to happen.

Contributor

cake-pie commented Sep 12, 2018

I don't buy the reasoning behind the decision to block any further mods that disable core features.

The recent change in default season length is accompanied by the announcement

[this] announces the intent that choosing non-standard timespans can and will cause issues

Well, equally, when a user applies mods to their game they should also beware issues, e.g. wonky balance, unexpected interactions or even outright breakages. This is true of any game, not just CDDA.

Why the difference in attitude?

Precluding mods from making even tiny code changes -- i.e. insert a check for some user-configurable option -- severely limits the range of what is possible.

In this case (no freezing), what could be done with very few lines of code would require a much larger json mod, and even that would not be possible if not for freezing point json setting to be added soon.

If we consider, say, Simplified Nutrition or Filthy Clothing, it's not clear those mods can even exist if they're not allowed to touch C++ code. For one, filthy clothing mechanics doesn't translate to something that can be straightforward jsonized and modded with purely json changes.

The inflexible philosophy of "core features can no longer be made optional" means that when Hypothetical Future Controversial Core Feature lands, it may not be possible to disable it within these limitations. The only recourse for people who don't want to play such a feature would be a hostile fork; I'd hate for that to happen.

@WizardOfOoo

This comment has been minimized.

Show comment
Hide comment
@WizardOfOoo

WizardOfOoo Sep 12, 2018

Contributor

There are already mods that disable core features. I don't understand why this one is so special. If the policy is to not allow those, then the other mods need to be removed.

Contributor

WizardOfOoo commented Sep 12, 2018

There are already mods that disable core features. I don't understand why this one is so special. If the policy is to not allow those, then the other mods need to be removed.

@kevingranade

This comment has been minimized.

Show comment
Hide comment
@kevingranade

kevingranade Sep 17, 2018

Member

Honestly I've been against mods to disable core game features all along, but at various times I've had co-maintainers that were very insistent on adding them, so I allowed myself to be overruled.

The freezing mechanics are also much more core to the game experience than filthy clothes, and much less difficult to get right than nutrition. I have a strong expectation that freezing is going to be sorted out within a reasonable timeframe, and once it is, there is no reason to make it optional, it's just part of the game, and more fundamentally, part of the game world. The situation is the same for the other mods, once they are ready to be enabled by default, the disablement options/mods will go away.

If you don't want to deal with freezing mechanics, there is a very simple option setting that should do it, which is enabling eternal seasons and setting your season to summer (spring or fall might work, I haven't checked). Especially with the ability to do this, I don't see a need for an option specific to the freezing mechanics.

I don't want to speak for @kevingranade, but I believe his objection to the previous attempt was because it was too intrusive and fragile the way it was done.

This is essentially correct. The real problem is that disabling core features is fundamentally fragile, it means there is not one game to test and debug anymore, there are dozens or hundreds of slightly different variants of the game. At some point a few years ago, we improved the options class to the point where it became trivial to add new options, over time that has resulted in more and more options being added, with little management of whether those options are a good idea. Various people have also taken that as a cue to carve out their own personal version of DDA within the main game, but at the same time, not take responsibility for maintaining said game version with testing and updates to code as needed. As a result, any non-default options tend to accumulate bugs over time, and it falls to core developers to maintain them, whether they think the option should exist or not.

As a result of this, I'm becoming more and more loath to allow new options to be added, and this one does not meet the bar for necessity.

As has been outlined here, you are free to make a mod that adjusts down the freezing point of various items, essentially disabling the mechanism, but in that case the mod is entirely external to dda, and places the burden of maintenance on the mod author, where it should be.

It would help either speed along the process of creating a mod, or shutting down a doomed endeavor early, if there would be some clarification of what was wrong with the previous attempt.

I have spoken at great length many times about why I'm resistant to expanding options within the codebase, it seems to make no discernible difference. As a result I've become more terse on the matter because in the past it hasn't helped. When I explain my reasoning people argue with it, if I don't explain my reasoning, people "get confused", but essentially don't cause further problems. As such the two approaches are essentially equivalent, and I follow one or the other based on how much free time I have at the moment. If you feel like cataloguing my responses on a particular topic, feel free, but as for me, I'm always too busy.

Why the difference in attitude? (between season length and freezing mechanic)

Because the horse is already out of the barn with respect to season length. If we had 365 day years and no option to change it, and someone proposed adding an option to adjust it, I'd deny it too. Since it's already an option, it needs to be treated differently.

In this case (no freezing), what could be done with very few lines of code would require a much larger json mod, and even that would not be possible if not for freezing point json setting to be added soon.

In general, we make as much configurable as possible in json definitions, and over time the trend is only toward increasing that. That is the mechanism whereby dda supports configurability. As for relative work, that just acknowledges that making it an option pushes work onto the core dda developers. If you want a mod, you're responsible for maintaining it.

Member

kevingranade commented Sep 17, 2018

Honestly I've been against mods to disable core game features all along, but at various times I've had co-maintainers that were very insistent on adding them, so I allowed myself to be overruled.

The freezing mechanics are also much more core to the game experience than filthy clothes, and much less difficult to get right than nutrition. I have a strong expectation that freezing is going to be sorted out within a reasonable timeframe, and once it is, there is no reason to make it optional, it's just part of the game, and more fundamentally, part of the game world. The situation is the same for the other mods, once they are ready to be enabled by default, the disablement options/mods will go away.

If you don't want to deal with freezing mechanics, there is a very simple option setting that should do it, which is enabling eternal seasons and setting your season to summer (spring or fall might work, I haven't checked). Especially with the ability to do this, I don't see a need for an option specific to the freezing mechanics.

I don't want to speak for @kevingranade, but I believe his objection to the previous attempt was because it was too intrusive and fragile the way it was done.

This is essentially correct. The real problem is that disabling core features is fundamentally fragile, it means there is not one game to test and debug anymore, there are dozens or hundreds of slightly different variants of the game. At some point a few years ago, we improved the options class to the point where it became trivial to add new options, over time that has resulted in more and more options being added, with little management of whether those options are a good idea. Various people have also taken that as a cue to carve out their own personal version of DDA within the main game, but at the same time, not take responsibility for maintaining said game version with testing and updates to code as needed. As a result, any non-default options tend to accumulate bugs over time, and it falls to core developers to maintain them, whether they think the option should exist or not.

As a result of this, I'm becoming more and more loath to allow new options to be added, and this one does not meet the bar for necessity.

As has been outlined here, you are free to make a mod that adjusts down the freezing point of various items, essentially disabling the mechanism, but in that case the mod is entirely external to dda, and places the burden of maintenance on the mod author, where it should be.

It would help either speed along the process of creating a mod, or shutting down a doomed endeavor early, if there would be some clarification of what was wrong with the previous attempt.

I have spoken at great length many times about why I'm resistant to expanding options within the codebase, it seems to make no discernible difference. As a result I've become more terse on the matter because in the past it hasn't helped. When I explain my reasoning people argue with it, if I don't explain my reasoning, people "get confused", but essentially don't cause further problems. As such the two approaches are essentially equivalent, and I follow one or the other based on how much free time I have at the moment. If you feel like cataloguing my responses on a particular topic, feel free, but as for me, I'm always too busy.

Why the difference in attitude? (between season length and freezing mechanic)

Because the horse is already out of the barn with respect to season length. If we had 365 day years and no option to change it, and someone proposed adding an option to adjust it, I'd deny it too. Since it's already an option, it needs to be treated differently.

In this case (no freezing), what could be done with very few lines of code would require a much larger json mod, and even that would not be possible if not for freezing point json setting to be added soon.

In general, we make as much configurable as possible in json definitions, and over time the trend is only toward increasing that. That is the mechanism whereby dda supports configurability. As for relative work, that just acknowledges that making it an option pushes work onto the core dda developers. If you want a mod, you're responsible for maintaining it.

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