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

Remove outdated blaze mod [CR] #17290

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
@mugling
Copy link
Contributor

commented Jun 21, 2016

Discussed elsewhere in a number of other issues with the summary being that the original author is maintaining a copy outside of this source tree and as a result the outdated version we ship is bit-rotting.

Patching code nobody ever uses isn't a productive use of time...

@illi-kun

This comment has been minimized.

Copy link
Member

commented Jun 21, 2016

What happens with save files with this mod enabled?

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2016

@illi-kun nearly everyone appears to be using the forum version. An alternative solution is #17256

@illi-kun

This comment has been minimized.

Copy link
Member

commented Jun 21, 2016

I'm not so sure about using of the forum version. The same process as we have for the obsolete items will be much better than current solution.

@Cyrano7

This comment has been minimized.

Copy link
Member

commented Jun 21, 2016

How does this effect the Tanks and other vehicles mod? Vehicle additions pack is listed as a dependency for it.

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2016

A selection of recent comments and commit messages:

The author is maintaining a separate tree that he isn't PRing

get rid of the mainlined version of the mod to lose the responsibility to maintain a very outdated version of the mod.

There is no reason to keep broken mods around.

few blazemod issues

I think blazemod still has...

Breaks blazemod due to...

Fix blaze mod

...but I'm guessing blazemod again?

A single ugly hack to make blazemod not produce errors on load

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2016

How does this effect the Tanks and other vehicles mod? Vehicle additions pack is listed as a dependency for it.

Does in practice it actually require it and if so the dependent content could be pushed to that mod?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jun 21, 2016

Instant removal like that will break saves. It's fine when going stable, but could be rather bad between two experimentals.
Honestly I'd rather rush the stable because last one is terribly outdated, but if that can't be done, #17256 is a bit safer than instant removal.

Keep in mind that blazemod uses some features that otherwise appear unused. Some of those should be supported, since they don't take much effort and are useful for modders. For example, MANUAL flag and tank treads.
It would be a good idea to bring them to core game, even if just as semi-placeholders. A tiny tracked vehicle and a ballista, for example.

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2016

I'm not so sure about using of the forum version.

Effectively the mod has been forked and we have the redundant copy so there isn't much option other than to drop support for it from master?

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2016

Keep in mind that blazemod uses some features that otherwise appear unused...It would be a good idea to bring them to core game, even if just as semi-placeholders

I was just typing a longer reply detailing that but you put it much better.

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2016

The tank mod also lacks a maintainer but it's not in such a bad shape and doesn't have a competing fork. We could push much of the required content into master then mark the included blazemod broken via #17256?

@BorkBorkGoesTheCode

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2016

Why not replace blazemod with the new version?

@Cyrano7

This comment has been minimized.

Copy link
Member

commented Jun 22, 2016

@BorkBorkGoesTheCode Part of it is someone needs to maintain it also. And there have not been any volunteers to update it even.

@BorkBorkGoesTheCode

This comment has been minimized.

Copy link
Contributor

commented Jun 22, 2016

I could do it, if I have extra time.

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2016

Why not replace blazemod with the new version?

I could do it, if I have extra time.

The original author is maintaining their own version, just not within this source tree. The two variants have diverged making the version in master a defacto fork of the authors. It's neither desirable nor reasonably practical to maintain the version in master in that circumstance.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jun 25, 2016

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jun 27, 2016

I don't consider mod content to have the same guarantees as mainline content.
I'd be fine to just merge this, and if someone complains, tell them, "tell blaze to coordinate with us"
I'm not going to argue strenuously for this right now since there seems to be some disagreement about it, but going forward we either:

  1. Officially lower support levels for mod content and feel free to drop them in situations like this.
  2. Raise level of support for mods, get more conservative about accepting them (blazemod was never high enough quality for me to sign off on supporting it indefinitely), and probably plan on evicting some of the content mods.
  3. ???

To address the immediate situation, it might be possible to add a mod option to obsolete the entire mod, meaning things it overrides don't get overridden, and things it creates get counted as obsolete, as in load errors are suppressed, but the entities just don't load.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

2 sounds nearly there. I'd change it a bit:
Instead of being conservative about accepting mods, we could require them to have a semi-active maintainer (unless they're too simple to require that) every x months.
So instead of dropping mods due to their low initial quality, we'd drop them due to lack of maintenance except what is necessary to avoid debugmsgs at load.

Someone should be cleaning them up once in a while. Or at least be ready to clean them up before 0.D.
0.D would be the perfect moment to drop all the unupdated mods. We could even have an issue for every big mod to find volunteers willing to update it. All the (content) mods without volunteers would get dropped.

@Night-Pryanik

This comment has been minimized.

Copy link
Member

commented Jun 27, 2016

we could require them to have a semi-active maintainer

You mean mod author or some random person to be maintainer?

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

You mean mod author or some random person to be maintainer?

Anyone, as long as we can depend on that person to actually do the job and not just volunteer and then forget.
Mod author would be preferable, but not necessary.

@BorkBorkGoesTheCode

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

I could update Icecoon's Weapons Pack.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jun 27, 2016

What's the rule for determining whether an author is active?
How does your proposal address the issue we're seeing right now?

I'm guessing blazemod would have been evicted a while back for lack of maintenance under this plan.

I'm a bit skeptical of giving people specific responsibilities like this. I suspect it will either be generally dysfunctional or increase communication overhead greatly.

Re: 0.D, it IS time to do a release, but the bugs aren't dropping, I'm guessing I'm going to have to do a feature freeze with more pending bugs than I'd like.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

0.D, it IS time to do a release, but the bugs aren't dropping

I PR'd a ton of fixes for 0.D bugs and most of the new ones are actually old bugs.
Feature freeze could be useful, but we're slowly getting there even without it. Just that we're moving slowly.

I'm a bit skeptical of giving people specific responsibilities like this.

Having "unowned" mods without anyone responsible places all the responsibility on anyone updating the core game.

How does your proposal address the issue we're seeing right now?

Regarding blazemod, nothing would change.

EDIT: Compared to your idea, that is. Blazemod being in mainline would change.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jun 27, 2016

I PR'd a ton of fixes for 0.D bugs and most of the new ones are actually old bugs.

I didn't mean to disparage anyone's work, I spoke carelessly, my apologies. What I meant was the number of bugs is dropping very slowly because new ones are being added at a fair clip in addition to a large number of old bugs being present. i.e. just slow as you say.

Having "unowned" mods without anyone responsible places all the responsibility on anyone updating the core game.

This is by design, mainlining mods means they are kept up to date as the core game is updated. We have no intention of maintaining a stable interface for mods to use, so it is only fair that when we adjust the core game in a way that breaks mods, we also adjust the mods to keep them working. If there's a reasonable third option I'm not aware of it. For context I'm explicitly following the model established by the linux kernel project of bringing drivers into the main repository so that core maintainers update them when the kernel needs to have a breaking change.

Even if someone is specifically on the hook for keeping mods up to date, people changing core game code are still going to be responsible for updating mods when they change the core code in a way that breaks them. It's simply not reasonable to make changes to the core game that break mods and just wait for a mod maintainer to come along and fix it. What your proposal does address is mods bitrotting over time, slowly going out of sync with the rest of the game or the like. I agree that these essentially abandoned mods should be removed instead of slowly becoming unusable.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

You seem to be forgetting that the tank mod is dependent on there being manual turrets, tracks, and some of the additional armor, right?

Removing the mod outright would be absurd.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Jun 27, 2016

manual turrets, tracks, and some of the additional armor

Which should really all be in the mainline anyway.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

Hell yes. That would unfuck the biggest, most annoying question I'd always see on Blaze's thread and the tankmod thread. If I had a dime for every person that couldn't wrap their head around using manual turrets because magic-powered AI turrets are the norm...

Tracks are a good idea too, but I assumed they remained mod content because they don't BEHAVE like tracks. They're just wheels by another name.

The suspension system and metal window armor (both used on the technical) are the armor you'd need to mainline, but the suspension system also has that flaw of being ARMOR by another name.

That reminds me, the bandit bulldozer also uses rams if I recall. You'll have to either mainline those, or edit them to use frame+armor or whatever the vanilla equivalent to a ram would be.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Jun 27, 2016

suspension system and metal window armor
rams

I see no reason these shouldn't be mainlined as well.

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2016

If you're going to maintain blaze mod, coordinate with blaze to get fixes pushed into his version

Emphasis mine but this is the correct approach for all who want to assist the mod - the version we have is an outdated fork whereas the author has both the original and most widely used copy. It's far from abandoned just not part of the core project.

Even if someone is specifically on the hook for keeping mods up to date, people changing core game code are still going to be responsible for updating mods when they change the core code in a way that breaks them

Exactly this. The maintenance cost is imposed on those who didn't volunteer and as a result our outdated fork consumes disproportionate resources for close to zero usage.

Raise level of support for mods, get more conservative about accepting them

Given that other developers are required to work on them so as to maintain compatibility it would seem reasonable to require at a minimum well structured JSON and at least cursory ongoing involvement. Authors who don't want this responsibility can (and indeed already do) ship third-party mods via the forums.

For context I'm explicitly following the model established by the linux kernel project of bringing drivers into the main repository so that core maintainers update them when the kernel needs to have a breaking change

Yes. If it's good quality code and another developers change breaks it they should update the code. If it it's bad quality code it should just be expunged at that point.

You seem to be forgetting that the tank mod is dependent on there being manual turrets, tracks, and some of the additional armor, right?

No this was brought up early on by @Cyrano7. Tank mod generally doesn't cause any maintenance headaches. To what extent could it be refactored not to depend upon blazemod - specifically what are the list of dependent items?

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

@Cyrano7 did mention it, but their concerns were never really addressed. As as I've said earlier, I always listed everything from Blazemod that the mod uses, that I can recall.

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2016

As as I've said earlier, I always listed everything from Blazemod that the mod uses, that I can recall.

Sorry it was a list of all the item id's or equivalent guidance on what needs doing as I'm not familiar with that mod and not breaking it is a reasonable objection to dropping the outdated blazemod.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

Ah, sorry about that. Will list it in a moment, going through it because I haven't looked at this crap of mine in ages.

EDIT: To list everything I'm aware of...

  • Several of the vehicles use armored tracks (part ID tread3). As mentioned, I'd assumed these remained mod-only because they don't behave like tracks actually should (unless this has changed since I ceased contributing).
  • The Atomic Mini-tank uses the "altered to be an AI turret" Browning M2 (part ID am2browning) and turret frame (part ID turretframe).
  • The Mobile Gun System and Infantry Fighting Vehicle also both use turret frames.
  • The Bandit Technical uses rebar plating (part ID rebar_plate) over the windows, and suspension (part ID spring_plate) on the wheels.
  • The Bandit Bulldozer also uses rebar plating, steel tracks (part ID tread2), and spiked rams (part ID ram_spiked).
  • Heavy farm tractors use the rubber tracks (part ID tread1).
  • The vehicles in general (the Mobile Gun System in particular) use the distinction between manual-fired and auto-turrets for balance, flavor, and other non-essential things. This last part is not essential to preserve, barring the auto-M2 as mentioned above.

So that's tread1, tread2, tread3, turretframe, am2browning, rebar_plate, spring_plate, and ram_spiked. Unless I missed anything else?

@BorkBorkGoesTheCode

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

I have contacted blaze. Blaze is willing to upload a copy of the updated mod to GitHub, but they don't know how to use GitHub.

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

I have contacted blaze. Blaze is willing to upload a copy of the updated mod to GitHub, but they don't know how to use GitHub.

Can they remember to avert the tabpocalyse? If they use Notepad++ for editing the files, they'll need to find auto-indent to disable it.

@BorkBorkGoesTheCode

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

Blaze is using Notepad++. Where is the auto-indent switch?

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

Settings, Preferences window, Misc. section, the auto-indent checkbox.

@BorkBorkGoesTheCode

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

Thank you chaosvolt

@chaosvolt

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2016

It's fine. I had that problem as well, for such an irksome feature it took me a while to find it.

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2016

Presuming I missed nothing out a summary of all the above discussion comes down to two requirements for inclusion in master:

The first is adequate code quality so as not to adversely affect other developers who will be obliged to contribute fixes to maintain compatibility with their changes. Mods shouldn't place excessive burdeon on others via poorly structured JSON or excessive hacks. Only the original mod author or current maintainer can ensure the code quality but they cannot be expected to maintain compatibility.

The second is avoidance of bitrot due to lack of popularity, lack of maintainer or separate distribution of competing forks. Popularity is decided by the community and the author decides on the distribution with competing forks lead by the original author defacto reducing any version in master to an outdated and unsupported copy.

The mod is popular and in that respect worthy of inclusion but code quality and maintenance need marked improvements. This can only be done by the original author as the mod is most definitively not abandoned. Pulling the latest copy doesn't resolve any of the above and having a github account is really an prerequisite for contributing code. We either need to improve the mod to meet the above or drop it entirely.

A gentle reminder for those that aren't aware that your comments are likely to be emailed or otherwise automatically distributed to many others. Q&A replies between two posters should for that reason be discouraged. I'd like to try and keep this PR on topic so please restrict discussion to technical argument/alternative courses of action.

@DeadLeaves

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2016

@BorkBorkGoesTheCode Well done contacting him. 👍

@Soyweiser

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2016

Just an FYI. I had no idea there was an more up to date version on the forums. I was using the ingame one, assuming it was the most updated one. (Having mods mainlined, and then still having different versions on the forums is a bit odd imho).

@mugling

This comment has been minimized.

Copy link
Contributor Author

commented Jul 1, 2016

Superseded by #17417

@mugling mugling closed this Jul 1, 2016

@kevingranade

This comment has been minimized.

Copy link
Member

commented Jul 2, 2016

Blaze is working with us to merge the mod properly, obsoleting this PR.

Thanks everyone for keeping the discussion productive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.