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
Allow Spell items to be converted to Scroll-type Consumable items #86
Comments
marked this issue as related to #87 |
Originally in GitLab by @ayyrickay Poking around the code, I see a couple of relevant methods in the 5e Actor Sheet: The Drag Item Start method looks like it has some logic for handling item drag events, but I'm guessing it's for items that are being dragged FROM the character sheet. It also has an onDrop Event that looks more promising. I'm thinking that the onDrop Event will need to check to see:
I'll have to check out the event to see if that info is available. But if it is, I could call that function to convert the spell object to a consumable, create a clone of the spell object assigned with the consumable data, and pass that to the inherited onDrop() method. I looked through the repo and there are a couple of things I can't find:
I can figure it out from the template file, but I figure there's always a chance I'll miss something or that the relationship won't be 1:1. |
My advice for the angle of attack here would be to separate the feature into two halves.
function createScrollFromSpell(spell: Item5e) : Item5e {
...
}
My advice would be to start with #1 first and ignore interaction workflows for now and simply start with the logic to do the transformation properly. You can find Level 1 through L9 scrolls in the Items5e compendium pack. My vision would be something that blends the data structure of those consumables with the actual spell details of the spell itself. |
Originally in GitLab by @ayyrickay Yeah, makes sense. Just looked at the spell and spell scroll objects, and it looks like there's plenty involved in that, haha. Is there a standard/preference for the UX of this? Two choices I'm already seeing are:
|
Good question, I think my intuition would be that the item name would be something like "Scroll of {Spell Name}", but "Spell Scroll: Spell Name" also seems good. For the description, I agree, there should probably be some component of the description that explains that it's a scroll and how scrolls work, while still containing the actual spell description. |
Originally in GitLab by @ayyrickay Here's a first stab, with a test for creating a Scroll of Light. I think the logic is straightforward enough, it's just a lot of boilerplate. I tried to use spread operators to make things a little more concise, while still leaving the code readable enough that you could come back and remember what's going on. I'm not sure where this type of file even goes and/or if there are better places for the boilerplate and helper functions in it. Happy to get feedback on the code quality and whether or not it's solving the problem in the meantime! :D |
I'm not able to open the file you attached - perhaps you could post the function as a snippet or just directly inline enclosed in triple backticks (`) My thinking is that either the converstion function can live as an |
Originally in GitLab by @ayyrickay
|
Originally in GitLab by @ayyrickay My bad. Posted here! |
Originally in GitLab by @ayyrickay Following up on this. Not sure if the silence is busyness or fear at the spaghetti code put in front of you, hahaha. 😅 Let me know if you're concerned about it, and I can explain my rational for writing it this way and/or improve it. Otherwise, I'll hold tight if it's just busy! :D |
Just me being busy mostly - I'm focused on the core 0.5.6 update now before cycling back to 5e for the next system update. I wonder if, as the "template" for each created scroll we could use the compendium items from the 5e items pack. For example, suppose we need to create a Level 3 spell scroll, we could get the basic scroll template for that item from the Items (SRD) compendium: const scrollTemplate = await game.packs.get("dnd5e.items").getEntry("hqVKZie7x9w3Kqds") |
Originally in GitLab by @ayyrickay Thanks for the response, and sorry to bug you! I'll hold tight in the future - and PS, if it helps, I'll take what I learn from this and try and document it for any future contributors! In any case, updated version (plus a makeshift unit test) below. I haven't hooked it into the actual game logic yet, so I'm mostly guessing it works, there might be errors that I need to work through.
|
Looking good to me. Here are a couple thoughts/requests: On Managing the Template Scroll IDsI don't think it's too much of a problem for us to hard-code the scroll IDs since we can work to make sure not to change the compendium IDs, but I would suggest that we at least expose them as something like CONFIG.DND5E.spellScrollIds = {
0: "Compendium.dnd5e.items.rQ6sO7HDWzqMhSI3",
1: "Compendium.dnd5e.items.9GSfMg0VOA2b4uFN",
...
} Then we can do Merging together the dataOnce you have the template, try the About the descriptionsI like the way you've structured the resulting description, with first a brief overview of what a scroll is, then the spell description, then further details about scroll usage - what I dont like is hard-coding that description text since that will be a blocker for localization efforts. I think we need some way to do this dynamically using the text in the scroll template and the text in the spell description. Maybe split on paragraphs and do like, template p1, spell description, remaining template paragraphs? |
Originally in GitLab by @ayyrickay Great points, and I definitely wasn't thinking about localization when I came up with the pared-down scroll text. I also updated the spell description to consider this - I'm assuming that enterprising DMs can go in and fiddle with the text once a general version has been generated. Don't have a great way to test the new methods, so just including the updated code here:
I put some TODOs in there, with the main thing being that the description generator could be a LITTLE clunky. I just wanted to make things like a random 4 (since indexOf would find the beginning of the first paragraph tag, so you need to compensate for that) less arbitrary, hence adding in a constant. I also have some general questions about unfamiliar methods. What is Similarly, does the |
Hey Ricky - this looks good, but I think the thing that concerns me the most in what you said is
That's definitely something we should work on so you can have a more effective test sandbox. My advice would be to create a world script (a .js file in your testing world) which you include in that world.json file as You can "inject" config directly into
|
Originally in GitLab by @ayyrickay I'm giving this a whirl and I think I'm running into some async issues:
I ran the debugger, and it's this line of code: It's pulling in |
Originally in GitLab by @ayyrickay Went ahead and increased some SetInterval details and confirmed the async problem. Not a problem for testing. Here's an updated script:
There is still an issue with the save. It feels like this should be able to be automated, but the save and attack bonus information seems to only exist in the spell scroll description. I can change it to Flat easily, but if the content gets changed to a different language, then there's not a reliable way to use the description to update the save data, plus I think it relies on Actor data as well. Seems like this might be a progress over perfection thing, since characters can calculate this on the fly, and DMs aren't going to allow a Wish spell to just be cast randomly, but wanted to let you know. |
Add quantity formula to group members, button to roll all quantities
we have a quite complete Spells SRD compendium which contains spell items
we also have "template" spell scrolls for levels 1 - 9 in the Items SRD
I'd love to have a utility function which takes a spell-type Item as an input and provides a properly configured spell scroll of that spell as an output
this could allow for us to drop spells either on the spellbook tab of the character sheet to add spells to the spellbook OR on the inventory tab to create spell scrolls of that spell
The text was updated successfully, but these errors were encountered: