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

Add support for musical instruments (scale off perception) #11468

Merged
merged 9 commits into from Mar 22, 2015

Conversation

Projects
None yet
7 participants
@Coolthulhu
Copy link
Contributor

commented Mar 5, 2015

Support for modable instruments.

Instruments are active items that produce sound and morale effect. This morale effect maxes out at (configurable) fun + fun_bonus * player->per_cur. Perception, because it's a bad stat, no other stat/skill makes sense here (plus grinding skills just by playing music would be horrid design) and because musical skill has a lot to do with hearing.

At low morale this can be a penalty - this is handled too.

Instruments consume some move points, so that they aren't totally free to use. If the player is too slow to play the instrument, it "turns off" to prevent players getting soft-locked and starving to death. They also turn off when the player goes to sleep, gets under water or drops the item.

Configurable stats are: sound volume, moves_cost, fun, fun_bonus, description_frequency and descriptions. Descriptions are strings that will be randomly picked from to describe the sound. description_frequency is the number of turns to wait between printing descriptions.

Instruments need to be wielded or worn to be played.

I added only one instrument (mostly for testing): harmonica with holder.

I had to add charges to the instrument, because otherwise it would disappear as soon as it got processed. Any better solution that doesn't involve having (1) in item name?

// Check for worn or wielded - no "floating"/bionic instruments for now
// TODO: Distinguish instruments played with hands and with mouth, consider encumbrance
const int inv_pos = p->get_item_position( it );
if( inv_pos > 0 || inv_pos == INT_MIN ) {

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 5, 2015

Contributor

inv_pos == 0 is inventory, too.

const double actions_per_turn = 100.0 / p->get_speed();
// How much does it cost (per player action) to play this instrument continuously
const double moves_per_action = moves_cost * actions_per_turn;
if( p->get_speed() / 2.0 < moves_per_action ) {

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 5, 2015

Contributor

You can shorten this to p->get_speed() * p->get_speed() < moves_cost * 200

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Mar 5, 2015

Author Contributor

But won't it be harder to understand?

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 5, 2015

Contributor

This is a very subjective thing, your view is as good as mine.

However, if I wanted to know whether a certain character (given speed) can play that instrument, I'd have to do track the variables, which are not used anywhere else, so they could be removed. But they help understanding the code, so it's fine.

Why actually this condition? It looks strange.

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Mar 5, 2015

Author Contributor

It's there to make the player bail out of playing the instrument if half of the turn would be used up just on playing the instrument. Without such a safeguard (or with lower proportions), player could be locked up trying to play the instrument while dangerous things happen around.

I think I'll change it to speed penalty, because it's safer, more obvious and doesn't penalize slow characters.

} else if( morale_effect < 0 && int(calendar::turn) % 10 ) {
// No musical skills = possible morale penalty
desc = _("You produce an annoying sound");
sounds::ambient_sound( p->posx(), p->posy(), volume, desc );

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 5, 2015

Contributor

That prints the sounds twice, it's printed below again.

void musical_instrument_actor::load( JsonObject &obj )
{
moves_cost = obj.get_int( "moves_cost", 25 );
volume = obj.get_int( "volume", 99 ); // Make it huge to alert the modder

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 5, 2015

Contributor

Make it huge to alert the modder

I don't get this. If it's mandatory, why not omit the default value and let the json parser throw an error when it's missing?

This comment has been minimized.

Copy link
@Coolthulhu

Coolthulhu Mar 5, 2015

Author Contributor

Didn't know parser drops errors on missing fields with no defaults.

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2015

When it costs moves, maybe a use function isn't the right thing to do. This can be very confusing for players, for example when they were used to one attack per turn (because their weapon attack speed matched their character speed). With instrument it's suddenly lower. And there is no obvious hint about this. I expect many bug reports.

Maybe this is better implemented as a player activity.

You could re-install the instruments from data/recycling/instruments.json, but you probably know this already.

harmonica with holder.

Ignore everything I said, I can now roleplay as zombie slashing Bob Dylan. My life is complete (-:

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2015

Yeah, that move cost thing is not ideal. I though about how to do it, but I have no better idea.

I don't want to make it an activity, because it is supposed to be used when reading books, waiting for soup to boil, traveling around etc.

Lowering speed maybe? Would make it less of a no-brainer when reading and crafting

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2015

Lowering speed maybe? Would make it less of a no-brainer when reading and crafting

I like that. At the end of player::recalc_speed_bonus, the speed is limited to at least 25% of the base speed, so there is no danger in soft locking the player. Item processing can currently not change player stats (see #11274, artifacts used to do this).

An idea (may not work):

  • Make it a generic item / armor (no charges, the canned food items are like this).
  • Change it's iuse to simply toggle item::active when invoked directly (t parameter is false).
  • Inside of player::suffer (maybe somewhere else?) check for active items with the iuse_musical_instrument type, invoke it from there with t set to true.
fun_bonus = obj.get_int( "fun_bonus", 0 );
description_frequency = obj.get_int( "description_frequency" );

JsonArray jarr = obj.get_array( "descriptions" );

This comment has been minimized.

Copy link
@BevapDin

BevapDin Mar 5, 2015

Contributor

You can use JsonObject::get_string_array to get the full array at once.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2015

Would effects be suited to adding slowdown? I'd add a "occupied" effect every turn with duration 1 or 2 and intensity dependent on some parameter. It would reduce speed based on intensity.

@BevapDin

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2015

Effects might work as well.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Mar 5, 2015

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Mar 5, 2015

OK, managed to do it using effects. It's a tiny bit ugly - I didn't find a way to re-apply an effect with lower intensity without overwriting current intensity, so I check for existing intensity before applying the new one. It is possible to play many instruments at once with no ill effects, but also no benefits.

@Snaaty

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2015

This looks great! Perhaps the playing an instrument effect should also tell that the morale is being improved by playing the instrument?

@KA101

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2015

On behalf of everyone who ever played an instrument (recorder & saxophone in my case), you build skill by playing it repeatedly, in horrible grindy fashion. It's boring as hell to do scales, Au Claire De La Lune, Imperium, The theme from Jurassic Park, etc repeatedly for hours on end.

But in this case, grindy gameplay would be eminently realistic. I'm asking that music capability be tied to a new skill, rather than permanently tied to one's general perceptual abilities.

@Rivet-the-Zombie

This comment has been minimized.

Copy link
Member

commented Mar 6, 2015

Is it really that important to measure if the character is actually good at playing?

If the effect remains the same, what's the purpose of a new (and very limited) skill?

@KA101

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2015

I'd imagine folks would want to play for NPCs (better musician = improved faction relations?) and/or feel that their music should improve with practice, no?

@KrazyTheFox

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2015

If the effect remains the same, what's the purpose of a new (and very limited) skill?

Perhaps it could actually result in a loss of morale until you have a sufficiently high skill? The amount scaling with skill, too (ex: 1 Skill = -10 morale, 5 skill = 5 morale, 10 skill = 20 morale).

@kevingranade

This comment has been minimized.

Copy link
Member

commented Mar 6, 2015

@KA101

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2015

I'd suggest morale-neutral at something like 3-4, so folks can feel like they're making progress. Minor stamina drain (varies by instrument) as playing does take energy.

@Coolthulhu

This comment has been minimized.

Copy link
Contributor Author

commented Mar 6, 2015

I picked a stat instead of skill, because in this case realism doesn't translate into good gameplay well.

With a skill, it would just mean all survivors become great musicians later on. A rock star zombie apocalypse may sound cool at first, but then you're left with the typical DDA midgame power creep, except now with extra purely time-dependent factor.
Unless the skill was tied to say, NPC skill training. Hard cap on practice, say at 3, rest from NPCs.

IRL skill grinding is bad for games, because here you just activate the grind and it "does itself". This adds nothing to the game, except yet another factor that distinguishes established survivors (who can wreck anything and anyone) from the new ones. In DDA, everyone becomes an independent, self-learned expert on everything after a while.

EDIT: Anyway, listening to own music for the n-th time would get boring, even with tons of skills - or maybe especially with tons of skills, because you don't have that reassuring progress and "noob gains".

@KA101

This comment has been minimized.

Copy link
Contributor

commented Mar 7, 2015

Anyway, listening to own music for the n-th time would get boring, even with tons of skills - or maybe especially with tons of skills, because you don't have that reassuring progress and "noob gains".

OK, then perhaps have the morale peak at 4-5, then drop off and plateau at 6ish? Depending on the PC's taste for experimentation, etc (which would be good to base on a stat/stats; weighted 2 or 3 parts PE to one part IN, perhaps?) the morale bonus gets better as the char enjoys changing things up and otherwise finds music rewarding.

George Carlin: "Jazz musicians are the only people I know who put in a full shift for pay, then go elsewhere and continue working for free."

Results to others would be largely skill-based, providing an incentive to practice and skill-up.

@kevingranade

This comment has been minimized.

Copy link
Member

commented Mar 9, 2015

George Carlin: "Jazz musicians are the only people I know who put in a full shift for pay, then go elsewhere and continue working for free."

Ha, he didn't know me, did he? :P

@KA101

This comment has been minimized.

Copy link
Contributor

commented Mar 9, 2015

No, I guess not, and sorry I missed the release. Megamerge GO.

Coolthulhu added some commits Mar 12, 2015

Merge branch 'master' into musical-instruments
Conflicts:
	src/item_factory.cpp
	src/iuse_actor.cpp
	src/iuse_actor.h

@kevingranade kevingranade merged commit 575c4c4 into CleverRaven:master Mar 22, 2015

1 check passed

default This has been rescheduled for testing as the 'master' branch has been updated.

@Coolthulhu Coolthulhu deleted the cataclysmbnteam:musical-instruments branch Apr 10, 2015

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.