Add a Shadow Field spell to the Magus class' repertoire #46910

ToxiClay · 2021-01-20T22:03:12Z

Summary

SUMMARY: Mods "Add a Shadow Field spell to the Magus class' repertoire"

Purpose of change

This PR adds a new spell to the Magus' bag of tricks: a shadow field! Cast it on a tile up to ten squares away, and the tile will be plunged into darkness.

Describe the solution

It took me longer than I care to admit (thank you @KorGgenT for the assistance), but I got it working on build 11380.

Values were kind of invented out of whole cloth, and I'm not sure I got them right.

Testing

Debugging and integration-testing performed on build 11380.

Additional context

Edit 21Jan: replaced category 'Content'

actual-nh · 2021-01-21T00:25:53Z

From looking at light_override (from fd_shadow and lightmap.cpp) - what happens if the target area is darker than the shadow field?

ToxiClay · 2021-01-21T02:02:01Z

From looking at light_override (from fd_shadow and lightmap.cpp) - what happens if the target area is darker than the shadow field?

That is....a very good question. Is there a way to override fd_shadow's definition to have a 0.0 light_override? I think NH is making the point that Shadow Field could make things lighter. That would be....bad.

actual-nh · 2021-01-21T02:16:26Z

From looking at light_override (from fd_shadow and lightmap.cpp) - what happens if the target area is darker than the shadow field?

That is....a very good question. Is there a way to override fd_shadow's definition to have a 0.0 light_override? I think NH is making the point that Shadow Field could make things lighter. That would be....bad.

That's what I suspect will happen. You could put a separate fd_darkness one (with 0.0 light_override) in Magiclysm/fields.json. (A nice addition to possible fields would be something like light_multiplier - or light_minimum and light_maximum.)

John-Candlebury · 2021-01-21T16:30:27Z

You should probably add a scroll to this one. And maybe also add it to the light manipulation book, which is named: Of light and falsehoods.

ToxiClay · 2021-01-21T17:21:29Z

You should probably add a scroll to this one. And maybe also add it to the light manipulation book, which is named: Of light and falsehoods.

There, that ought to do it. Thanks for the heads up.

ToxiClay · 2021-01-21T17:29:20Z

This should address @actual-nh's concern with the previous fd_shadow field possibly causing a light increase.

actual-nh · 2021-01-21T18:04:24Z

Technically, the spell should now be called "Darkness Field", but that's probably just me being compulsive...

ToxiClay · 2021-01-21T18:05:10Z

Technically, the spell should now be called "Darkness Field", but that's probably just me being compulsive...

I would have left the field definition as fd_shadow if I weren't 95% sure that would cause something to snap in half.

actual-nh · 2021-01-21T18:17:06Z

I would have left the field definition as fd_shadow if I weren't 95% sure that would cause something to snap in half.

You will need to put fd_darkness in Magiclysm/fields.json - there currently is no fd_darkness. (And fd_shadow is in main, not the mod, so modifying it to light_override 0 might be considered out of scope for a mod PR...)

ToxiClay · 2021-01-21T18:57:16Z

I would have left the field definition as fd_shadow if I weren't 95% sure that would cause something to snap in half.

You will need to put fd_darkness in Magiclysm/fields.json - there currently is no fd_darkness. (And fd_shadow is in main, not the mod, so modifying it to light_override 0 might be considered out of scope for a mod PR...)

~~What the hell-- I swear to God I did, and then Github ate my commit. Goddammit.~~

Huh. I guess I didn't.

data/mods/Magiclysm/field.json

data/mods/Magiclysm/Spells/magus.json

ToxiClay · 2021-01-21T21:55:57Z

Hm. Can I actually change the name to `darkness1 in the field definition, or did I do an oopsie by committing that?

 "intensity_levels": [ { "name": "shadow", "light_override": 3.7 } ]

vs

 "intensity_levels": [ { "name": "darkness", "light_override": 0.0 } ]

actual-nh · 2021-01-21T22:18:10Z

Hm. Can I actually change the name to `darkness1 in the field definition, or did I do an oopsie by committing that?

That is the name not for the field itself, but for that level of intensity. (Check the end of JSON_INFO.md.)

ToxiClay · 2021-01-21T22:31:10Z

Hm. Can I actually change the name to `darkness1 in the field definition, or did I do an oopsie by committing that?

That is the name not for the field itself, but for that level of intensity. (Check the end of JSON_INFO.md.)

Ahhh, gotcha.

KorGgenT

it just needs a minor tweak to the spell, but otherwise it looks good.

data/mods/Magiclysm/Spells/magus.json

KorGgenT · 2021-01-24T17:23:13Z

data/mods/Magiclysm/field.json

+    "id": "fd_darkness",
+    "type": "field_type",
+    "intensity_levels": [ { "name": "darkness", "light_override": 0.0 } ],
+    "half_life": "10 seconds",


just a heads up that the duration of the spell adds to the half-life, it doesn't override it.

I'm not entirely sure how to change my values with that in mind, or what that would mean in practice and context. Do you have any suggestions off-hand?

Oh, okay, I think I see what you're saying. My intention was for the first level to give a minute of darkness, and the 30th to give fifteen. But if the duration adds to the half_life, that means that at first level, one minute after casting, the field is down to half intensity, then a quarter intensity after two minutes, and an eighth intensity after four.

So I should cut all my values in half.

actual-nh · 2021-01-24T19:43:43Z

It is very unhelpful of github to just display the conflicting file's name. How does one look at what's conflicting?

actual-nh · 2021-01-24T19:47:15Z

Looks like an update from master is needed - it's the Forge modification adding a new field also.

… It also adds the spell to the relevant book and creates a new scroll, and then seeds that scroll into the world. Creating this spell required the creation of a new field, fd_darkness, which is patterned off of the base game's fd_shadow.

ToxiClay · 2021-01-24T20:00:46Z

Looks like an update from master is needed - it's the Forge modification adding a new field also.

Yup. I was notified of that while rebasing. I've fixed it up, and will be force-pushing shortly.

Side note: I finally understand git rebase!

actual-nh · 2021-01-24T20:06:54Z

Side note: I finally understand git rebase!

I'm not sure I do. And exactly what is meant (from the git command-line viewpoint) by "force-pushing"? Doing it not-fast-forward? Thanks!

ToxiClay · 2021-01-24T20:24:40Z

Side note: I finally understand git rebase!

I'm not sure I do.

OK. You have to make a new branch to work on a feature, right? You start making your commits at a certain point in time, but upstream/master keeps chugging along.

          A---B---C You
         /
    D---E---F---G upstream/master

When you're ready to PR your changes, if you just open a PR, you'd be asking to pull in commits A, B, and C, but you'll be throwing away F and G, because they don't exist in your branch.

What "rebasing" does is quite simply just that: it "re-bases" your branch. Instead of the base of your branch being E, Git picks up all the changes you made, changes the base of your branch to point at G, and then replays your commits onto the new base:

                  A'--B'--C' You
                 /
    D---E---F---G upstream/master

Git will warn you if there are conflicts -- if someone in F or G changed one or more of the same files you did. You'll be asked to investigate each conflicting file and resolve the discrepancy. If you rebase interactively, you'll also be given the opportunity to change the order of your commits, squash commits together, or change the commit messages.

At the end of the rebase, there's a smooth, linear history, and it's exactly as if you'd been working from master all along. It's better than trying to do a merge, which might lose your local commits.

And exactly what is meant (from the git command-line viewpoint) by "force-pushing"? Doing it not-fast-forward? Thanks!

A normal git push works in fast-forward mode: that is, if there's a shared history in the commit log, Git can just update some pointers and the history is still nice and linear.

But if the histories don't match -- if you've rebased a branch locally and are trying to push it, for example -- Git won't let you do a fast-forward. You'll have to force it, which wipes out anything that might be in the remote repo that you don't have locally. You have to do this explicitly, or Git will fail:

$ git push
To https://github.com/ToxiClay/Cataclysm-DDA.git
 ! [rejected]              magiclysm-field-shadows -> magiclysm-field-shadows (non-fast-forward)
error: failed to push some refs to 'https://github.com/ToxiClay/Cataclysm-DDA.git'
hint: Updates were rejected because the tip of your current branch is behind
hint: its remote counterpart. Integrate the remote changes (e.g.
hint: 'git pull ...') before pushing again.
hint: See the 'Note about fast-forwards' in 'git push --help' for details.

The key here is "because the tip of your current branch is behind its remote counterpart" -- that is, Git is telling you that changes exist in the remote that are "in the future of" your local repo, and you will lose them if you continue.

KorGgenT · 2021-02-07T06:58:39Z

works! ish. maybe needs a small fix in the tile drawing code or something but that's out of scope of this pr

…46910) Creating this spell required the creation of a new field, fd_darkness, which is patterned off of the base game's fd_shadow.

ToxiClay requested a review from KorGgenT as a code owner January 20, 2021 22:03

anothersimulacrum added [JSON] Changes (can be) made in JSON Mods: Magiclysm Anything to do with the Magiclysm mod labels Jan 20, 2021

ToxiClay force-pushed the magiclysm-field-shadows branch from b9a4fff to 88bd1ab Compare January 21, 2021 17:47

actual-nh reviewed Jan 21, 2021

View reviewed changes

data/mods/Magiclysm/field.json Outdated Show resolved Hide resolved

actual-nh reviewed Jan 21, 2021

View reviewed changes

data/mods/Magiclysm/Spells/magus.json Outdated Show resolved Hide resolved

KorGgenT reviewed Jan 24, 2021

View reviewed changes

ToxiClay force-pushed the magiclysm-field-shadows branch from 58688ae to ac5093d Compare January 24, 2021 20:01

KorGgenT merged commit bbda382 into CleverRaven:master Feb 7, 2021

actual-nh mentioned this pull request Feb 24, 2021

Unhardcode bio_flashlight and bio_tattoo_led #47714

Merged

actual-nh mentioned this pull request Apr 15, 2021

[CR] Add CAMOUFLAGE monster flag #48340

Merged

5 tasks

ToxiClay deleted the magiclysm-field-shadows branch July 11, 2021 14:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a Shadow Field spell to the Magus class' repertoire #46910

Add a Shadow Field spell to the Magus class' repertoire #46910

ToxiClay commented Jan 20, 2021 •

edited

Loading

actual-nh commented Jan 21, 2021

ToxiClay commented Jan 21, 2021

actual-nh commented Jan 21, 2021

John-Candlebury commented Jan 21, 2021 •

edited

Loading

ToxiClay commented Jan 21, 2021

ToxiClay commented Jan 21, 2021

actual-nh commented Jan 21, 2021

ToxiClay commented Jan 21, 2021

actual-nh commented Jan 21, 2021

ToxiClay commented Jan 21, 2021 •

edited

Loading

ToxiClay commented Jan 21, 2021 •

edited

Loading

actual-nh commented Jan 21, 2021

ToxiClay commented Jan 21, 2021

KorGgenT left a comment

KorGgenT Jan 24, 2021

ToxiClay Jan 24, 2021

ToxiClay Jan 24, 2021

actual-nh commented Jan 24, 2021

actual-nh commented Jan 24, 2021

ToxiClay commented Jan 24, 2021

actual-nh commented Jan 24, 2021

ToxiClay commented Jan 24, 2021

KorGgenT commented Feb 7, 2021

Add a Shadow Field spell to the Magus class' repertoire #46910

Add a Shadow Field spell to the Magus class' repertoire #46910

Conversation

ToxiClay commented Jan 20, 2021 • edited Loading

Summary

Purpose of change

Describe the solution

Testing

Additional context

actual-nh commented Jan 21, 2021

ToxiClay commented Jan 21, 2021

actual-nh commented Jan 21, 2021

John-Candlebury commented Jan 21, 2021 • edited Loading

ToxiClay commented Jan 21, 2021

ToxiClay commented Jan 21, 2021

actual-nh commented Jan 21, 2021

ToxiClay commented Jan 21, 2021

actual-nh commented Jan 21, 2021

ToxiClay commented Jan 21, 2021 • edited Loading

ToxiClay commented Jan 21, 2021 • edited Loading

actual-nh commented Jan 21, 2021

ToxiClay commented Jan 21, 2021

KorGgenT left a comment

Choose a reason for hiding this comment

KorGgenT Jan 24, 2021

Choose a reason for hiding this comment

ToxiClay Jan 24, 2021

Choose a reason for hiding this comment

ToxiClay Jan 24, 2021

Choose a reason for hiding this comment

actual-nh commented Jan 24, 2021

actual-nh commented Jan 24, 2021

ToxiClay commented Jan 24, 2021

actual-nh commented Jan 24, 2021

ToxiClay commented Jan 24, 2021

KorGgenT commented Feb 7, 2021

ToxiClay commented Jan 20, 2021 •

edited

Loading

John-Candlebury commented Jan 21, 2021 •

edited

Loading

ToxiClay commented Jan 21, 2021 •

edited

Loading

ToxiClay commented Jan 21, 2021 •

edited

Loading