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

[Magiclysm] Add bag of infite knives, allow to specify more conditions for artifact recharging #47610

Merged
merged 8 commits into from
Sep 24, 2021

Conversation

Salty-Panda
Copy link
Contributor

@Salty-Panda Salty-Panda commented Feb 19, 2021

Summary

Mods "[Magiclysm] Add bag of infinite knives"

Purpose of change

I wanted it.

Describe the solution

Added recharge_condition::worn, wielded, held to artifacts. It works like periodic, but require the item to be worn/wielded
Added the bag, it casts a spell that summons one fake item, a throwing knife. The knife is kinda lackluster, in the future the enchantments will be able to modify skills though, so it shall be updated.
Changed description and reduced the price ( KorGenT blocked wearing multiple rings at once recently ) of an existing ring that was doing something similar.

Describe alternatives you've considered

Adding the possibility to modify skills via enchantment, but KorGenT already has specific plans on how to do it.

Adding a greater version that randomly summons variant dealing electric/pure/more piercing/acid damage. would require cpp support too.

Testing

Spawned, used

Additional context

Active artifacts that cast spells and reload on their own could be abused by wearing a few "Ring of cast strong spell" in a backpack and just equipping them when needed in place of a staple + STAT.
Forcing PC to wear items in order to recharge them somewhat reduces the problem.
Also, that ring was outrageously expensive considering the uselessness of throwing knives. It could only summon a few, required an hour to recharge a single-use, and was half again as expensive as a ring with a permanent 1 STR bonus.

@anothersimulacrum anothersimulacrum added 0.F String Freeze [JSON] Changes (can be) made in JSON Mods: Magiclysm Anything to do with the Magiclysm mod labels Feb 23, 2021
Copy link
Member

@KorGgenT KorGgenT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the artifact code, worn should not be a "charge type" it should be a "prerequisite"; that way you can have worn be required for any charge type, isntead of adding worn enums forr each charge type. see also: how i implemented artifact::has

Copy link
Member

@KorGgenT KorGgenT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the artifact code, worn should not be a "charge type" it should be a "prerequisite"; that way you can have worn be required for any charge type, isntead of adding worn enums forr each charge type. see also: how i implemented artifact::has

@Salty-Panda
Copy link
Contributor Author

in the artifact code, worn should not be a "charge type" it should be a "prerequisite"; that way you can have worn be required for any charge type, instead of adding worn enums for each charge type. see also: how i implemented artifact::has

So I should add a struct to relic.h

enum relic_recharge_condition {
    solar_sunny,
    worn,
    num
};

and to recharging function some check before the switch with charge.type?

    if(charge.requirement == relic_recharge_condition::worn){
        if( ! ( item.is_wield || item.is_held ) ) ) {
            return;
        }
    }

Essentially make it work in a way similar to how "has" and "condition" behave?

@Salty-Panda Salty-Panda marked this pull request as draft April 9, 2021 19:23
@Salty-Panda
Copy link
Contributor Author

NOTE: Use WONDER flag, set effect to none, specify variants in other_effects

@KorGgenT
Copy link
Member

KorGgenT commented Jul 11, 2021

So I should add a struct to relic.h [...]

yes, exactly. sorry it took me so long to get back to your question!

@Salty-Panda
Copy link
Contributor Author

@KorGgenT done and tested

@Salty-Panda Salty-Panda marked this pull request as ready for review August 15, 2021 15:05
@Salty-Panda
Copy link
Contributor Author

Forgot to add spawning for POC item though, will add to some item groups today

@Salty-Panda
Copy link
Contributor Author

Added a few variants using WONDER flag, sadly you can't use copy-from in spells.
Increased price since it's more powerful overall and added it to itemgroups

src/relic.cpp Outdated Show resolved Hide resolved
@Salty-Panda
Copy link
Contributor Author

The only failed test is vehicle_turrets_test.cpp
AFAIK, this PR is finally done and ready to review

@Salty-Panda Salty-Panda changed the title [Magiclysm] Add bag of infite knives [Magiclysm] Add bag of infite knives, allow to specify more conditions for artifact recharging Aug 29, 2021
@KorGgenT
Copy link
Member

KorGgenT commented Aug 30, 2021

i don't have the time to review this tonight, but have you tested how loading previously saved artifacts will work with your change? it's important that they be able to load without errors. and it's better that you test that before i test it and bounce off of it for that exact reason, it'll save time

@Salty-Panda
Copy link
Contributor Author

Loaded save with two artifacts, Dim Sky(2/2) and Fire Rain(1/1), used both and both refilled to full.
No errors, no visible debug messages

@KorGgenT KorGgenT merged commit 11e9ead into CleverRaven:master Sep 24, 2021
@Salty-Panda Salty-Panda deleted the add_infite_knife_bag branch September 24, 2021 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[JSON] Changes (can be) made in JSON Mods: Magiclysm Anything to do with the Magiclysm mod
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants