Civcraft / Citadel

Do not open issues here; open them on the maintained fork @ DevotedMC
https://github.com/DevotedMC/Citadel
BSD 3-Clause "New" or "Revised" License
6 stars 23 forks source link

Acid maturation bifurcation #148

Closed ProgrammerDan closed 8 years ago

ProgrammerDan commented 8 years ago

Don't merge -- for discussion.

One thing that comes up when using maturation is that having acid time the same as block maturation is ... well, really inconvenient.

Let's say you want to have block maturation of 20 minutes, but acid time of 1 day. Can't. Have to have acid time of 20 minutes, which might as well be instant break.

In any case, to have usable acid w/ maturation, they need to be separate timers.

This pull represents that change. I'm putting it here to facilitate discussion and make sure I covered all my bases in the change. It compiles but I haven't tested it -- that's next up.

@rourke750 when you get a few spare cycles, would love you to look over and advise.

ttk2 commented 8 years ago

So this is about decoupling acid time from reinforcement maturation time? Is anyone seriously using reinforcement maturation right now?

Maxopoly commented 8 years ago

Well we could use it.

ProgrammerDan commented 8 years ago

Yeah Devoted is, we're probably the only ones. Been doing like a line or two on this every few days, this is closest to completion I've been.

Had a bunch of time today (sick family day, hah) so hit up a bunch of my 'open work' list stuff.

It's cool if Civ isn't interested in Maturation/Acid flexibility, but I think at least having it "right" in the code is probably a good move overall. Maturation is an interesting mechanic, but it's horribly broken when coupled with Acid time.

ttk2 commented 8 years ago

I always thought that they should be coupled because it relates time to place a block to time to remove it.

ProgrammerDan commented 8 years ago

It makes sense when there isn't damage scaling active.

However we've discovered on Devoted that if damage scaling is active, it's absolutely terrible. To have sensible maturation times (like 15 mins or something) where newly reinforced blocks are vulnerable, you also have acid blocks that, effectively, instant-mature. This isn't good, as it means all reinforcements are always vulnerable with damage scaling during maturation.

Or, you wind up with what we've had to do for now, where the two are tied and diamond level reinforcements are weaker then iron for half a day, practically.

ttk2 commented 8 years ago

then decouple them I guess.

On Mon, Nov 23, 2015 at 7:45 AM, Daniel Boston notifications@github.com wrote:

It makes sense when there isn't damage scaling active.

However we've discovered on Devoted that if damage scaling is active, it's absolutely terrible. To have sensible maturation times (like 15 mins or something) where newly reinforced blocks are vulnerable, you also have acid blocks that, effectively, instant-mature. This isn't good, as it means all reinforcements are always vulnerable with damage scaling during maturation.

Or, you wind up with what we've had to do for now, where the two are tied and diamond level reinforcements are weaker then iron for half a day, practically.

— Reply to this email directly or view it on GitHub https://github.com/Civcraft/Citadel/pull/148#issuecomment-158936461.

rourke750 commented 8 years ago

ok to test

rourke750 commented 8 years ago

@ProgrammerDan I broken your pull with my recent commit. Can you rebase and you're good to merge.

ProgrammerDan commented 8 years ago

Yep, gotta do more testing so don't want to merge this yet anyway. Layering in the new field wasn't terrible per-sey, but I'll do some deep DB inspection before I feel comfortable merging.

So the failure is good for the moment :D . Wanted to test the waters to see if there would be any strong objection to this, seems there isn't any held objection so I'll finish out the testing.