mivalsten / ddb-dm-screen

MIT License
30 stars 10 forks source link

Get help from someone competent #4

Open mivalsten opened 5 years ago

tbbstny commented 5 years ago

To achieve what?

mivalsten commented 5 years ago

There is issue #9 that requires someone competent for example, but in a broader sense this issue was a self-depreciating joke made after realisation that i'm not at all good at programming. All and all, code I wrote is a mess and requires a lot of cleamup, commenting and such, all help is apreciated.

tbbstny commented 5 years ago

I would like to help where I can. I played w/ my local script in Tamper Monkey a bit today, and think it's pretty slick. I personally would like to see alignment, Hit Dice, and maybe ideals, bonds, flaws (and I forth I can't think of right this sec.). I also noticed a section of hidden iFrames that load all the character views - what is the intent of that? Something you have plans to expand on? I guess I'm wondering what the bigger picture you are looking for - keeping with the current stat table, moving to iframes, combo of the two?

mivalsten commented 5 years ago

Iframes are a hacky way to get proper AC (at least 3 classes have unarmored AC, and races too), but I'm more and more inclined to rewrite whole script to take computed values from there. Probably for version 2.0. Alternative would be to just expand those iframes and overlay campaign manager to just show the data, but that sounds cheap. :D

jfabre commented 5 years ago

If you're patient a little, I will submit a pull request very soon. I've refactored a lot of the code already. I first removed the iframe logic, because yes it's hacky but I now think that is the only way to go if we don't want to have to rewrite all the dnd logic.

I would be easier just to scrape every computed values from the character sheet than rewrite all the logic. I don't understand why this isn't already computed in the json object, but it doesn't matter.

I'm putting the iframe logic back for AC, then I might expand it to deal with the hp bug.

mivalsten commented 5 years ago

There is another HP bug? The fastest way for dealing with those would be to just take them from the iframes, same as AC.

jfabre commented 5 years ago

My dwarf player def doesn't have the right hp amount, I think he's missing the +1 on every level.

jfabre commented 5 years ago

I think exactly the same, let's not rewrite everything.

mivalsten commented 5 years ago

On the other hand, rewriting D&D rules allowed me to use js class in real life. :D But yeah, just parsing the website should be more consistent. Note on that, due to iframe size, it's loading mobile version with different structure and div names than desktop one, that's a bit of pain in the ass to use.

jfabre commented 5 years ago

So the iframe structure/data is not the same on mobile vs desktop? I noticed you were using mobile, indeed. So the scraping would have to account for both case, is that it?

mivalsten commented 5 years ago

No, I think we should just stick to the mobile version, from what I was able to check, it's a bit easier to scrape. I'll make a branch for 2.0 to keep working on together. https://github.com/mivalsten/ddb-dm-screen/tree/V2.0-full-scrape

jfabre commented 5 years ago

Ok, I only need to scrape the passive skills, display them in the table and I will submit a pull request.

jfabre commented 5 years ago

Aight, I don't know what to do anymore... :)

Hope this helps! if you need help with something else I'll be watching this repo for updates. I do want a better presentation for my own campaign, but at least I have something functional. Maybe languages would be useful to scrape too?

mivalsten commented 5 years ago

@jfabre Thanks a lot for your help, I'll merge v2 to master this afternoon. One thing left to figure out is how to display skills, but there is an issue for that too.

Not sure about languages, this does not come handy a lot, and then GM can just open full character sheet. One important functionality would be to periodically update iframe page and update HP and such, i'll open issue for that too.

mivalsten commented 5 years ago

@jfabre I have created new branch with my improvement that drops json requirements completely. https://github.com/mivalsten/ddb-dm-screen/tree/drop-json-requests

The problem is, to many calls to other people characters blocks you from accessing them at all for a short period of time (presumably 1 hour) which is not ideal.

jfabre commented 5 years ago

Makes senses. If you can split the amount of call to their server by 2 and simplify the code, it's a good thing. I would probably avoid the auto-refresh feature for the same reason. Even though it sounds great, it might generate an undesired amount of traffic to DDB. People can always just refresh the webpage if they need the latest info.

mivalsten commented 5 years ago

I'm also having an issue with iframes not loading on time in campaigns with a lot of characters, is there a way to actually know when the page is loaded? Normally it's done with document.ready, but this does not work with iframes. Ideas?

jfabre commented 5 years ago

I haven't read everything, but you should have a good idea of what you can do with this:

https://stackoverflow.com/questions/3142837/capture-iframe-load-complete-event

mivalsten commented 5 years ago

Thanks, I have implemented onload in newest commit. There is room for improvement, ex. helper function that tries to access some element of the iframe and if it's not present yet it waits for 1000ms and tries again. If it succeeds it calls render or dies after 20 tries or so.

mivalsten commented 5 years ago

I have added aformentioned prerender function that ensures, that iframe had loaded properly before rendering data. The issue was, that after iframe loaded completely (and fired onload event) it still needed to calculate actual character fields as all thowe were done in frontend.

lothsun commented 5 years ago

@jfabre I'm racking my brains out here trying to get the spell Save DC added to the script. Any chance you could assist with that? I was able to get the initiative done but I'm far from a jquery master.

jfabre commented 5 years ago

@lothsun where is it in the sheet?

tbbstny commented 5 years ago

@lothsun will this work? In looking at the markup, I don't see a specific class or id that will select the DC. I notice there is an outer element (.ct-spells-level-castinginfo) that includes all three items - modifier, attack and DC. All three items are use class `.ct-tooltip.ct-spells-level-castinginfo.item`. So the selector in the snippet below fetches all three, picks the 3rd one, and grabs the text value.

$('.ct-spells-level-casting__info .ct-tooltip.ct-spells-level-casting__info-item').eq(2).text()
lothsun commented 5 years ago

I was finally able to get it to work. It doesn’t get loaded into the page until you click the button that shows the info. Took me a minute to figure out that I needed that but I finally got it. I’m working on a redesign that gives it better UI/UX and looks more natural to the website.

lothsun commented 5 years ago

I guess I’m also indirectly accomplishing issue #3. As I’m commenting my code and going back and commenting the rest of the code that I understand.