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

Migrate M47 from Magiclysm with Generic Guns active #37659

Merged
merged 2 commits into from Feb 9, 2020
Merged

Migrate M47 from Magiclysm with Generic Guns active #37659

merged 2 commits into from Feb 9, 2020

Conversation

ghost
Copy link

@ghost ghost commented Feb 3, 2020

Summary

SUMMARY: Bugfixes "Fix Magiclysm guns showing when using Generic Guns"

Purpose of change

Guns from Magiclysm can show up when also using Generic Guns, and keep their original ammo types rather than being generalized.

Describe the solution

Add gunblade and M47A1 to GG migration list.

Describe alternatives you've considered

None.

Testing

Tested with just GG active: No visible load errors(blacklisting the guns instead does cause a load error).
Tested with both active: Guns show as original when first started, reloading the world causes migration regardless of mod loading order.

Additional context

@esotericist said to put this fix out now so it isn't forgotten for after the migration code is fixed to migrate on first use of a world.

@ghost ghost requested a review from esotericist February 3, 2020 01:23
@KorGgenT KorGgenT added <Bugfix> This is a fix for a bug (or closes open issue) Items / Item Actions / Item Qualities Items and how they work and interact Mods: Magiclysm Anything to do with the Magiclysm mod labels Feb 3, 2020
@I-am-Erk
Copy link
Member

I-am-Erk commented Feb 3, 2020

I'm not sure about this M47A1 dealy, but isn't the gunblade a fairly specific weapon to magiclysm? generic guns isn't about not getting any guns to play with, it's about making the choices simple. You might just want to change the accepted ammo type in GG.

@esotericist
Copy link
Contributor

I'm not sure about this M47A1 dealy, but isn't the gunblade a fairly specific weapon to magiclysm? generic guns isn't about not getting any guns to play with, it's about making the choices simple. You might just want to change the accepted ammo type in GG.

For the M47A1: It's literally just a revolver that kiiiiiinda sucks in real life made to suck slightly less through the use of magic, and the ultimate effect gets you a gun that works exactly the same as the basic revolver in Generic Guns (due to the loss of differentiation of ammo types). So it migrating to the GG revolver is the right call.

As for the gunblade having its stats changed? This isn't actually simple in our current mod framework; if generic guns refers to content from magiclysm in any form other than migration, it results in errors if the player has generic guns and not magiclysm.

Fixing in that fashion will require code support that I honestly have no idea how to do, or it will require a compatibility mod for integration. I don't think either is appropriate in the lead-up to 0.E, but I would like to see something better after.

@KorGgenT
Copy link
Member

KorGgenT commented Feb 3, 2020

the m47A1 is basically "i can use a bunch of different ammo" so that makes sense to migrate, but you do have a good point. I don't know how it would work to migrate the ammo of an item from a different mod such that they don't have a real dependency on each other though

@I-am-Erk
Copy link
Member

I-am-Erk commented Feb 3, 2020

I'd suggest just migrating gunblade to gunblade_generic perhaps?

@esotericist
Copy link
Contributor

I'd suggest just migrating gunblade to gunblade_generic perhaps?

How would that work?

Are you wanting to put a gunblade_generic in magiclysm that uses the generic gun ammo type? That would error if generic guns is absent.

Are you wanting to put a gunblade_generic definition in Generic Guns? I'm not prepared to believe Generic Guns has a responsibility to have generic versions of weapons from content mods.

@esotericist
Copy link
Contributor

So on review, this definitely needs a code solution for the gunblade; it should be possible to have its ammo usage automatically migrated without us needing to specify anything in the generic guns mod at all. I'm looking into that now.

the techno medusa migrating to a revolver is still the way to go, imo.

@ghost ghost changed the title Fix Magiclysm guns showing with Generic Guns active Migrate M47 from Magiclysm with Generic Guns active Feb 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
<Bugfix> This is a fix for a bug (or closes open issue) Items / Item Actions / Item Qualities Items and how they work and interact Mods: Magiclysm Anything to do with the Magiclysm mod
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants