death-save / pin-cushion

Adds additional functionality around Map Pins
GNU General Public License v3.0
7 stars 18 forks source link

Some new feature Backgroundless Pins, Journal Image by default, Journal Image Thumbnail and compatibility with FVTT 9 #71

Closed p4535992 closed 2 years ago

p4535992 commented 3 years ago

Here I have tried to reduce the changes as much as possible.

I know for a fact that many people use the following three modules always together "pin-cushion", "foundryvtt-journal-thumbnail", "FVTT-Backgroundless-Pins" the last two have not been updated for almost 10 months and it made sense to me (also to avoid a module conflict) update and integrate these features.

I've checked the licenses and it should be fine.

The combination of these features is in great demand in investigative games where you want to put objects in the room.

P.S. I add in the PR a prettier configuration based on what I have seen other foundry developers use, consider using it (or if you can please put your own on git) so as to synchronize us when we develop something and prevent git from going crazy with whitespace, tabs, etc.

eclarke12 commented 3 years ago

Thanks, I have done a quick review and it looks pretty good! I will pull and make some changes and aim to release an update later this week.

eclarke12 commented 3 years ago

@p4535992 I haven't forgotten about this. I'm just finishing up a big CUB update then I will look at merging this PR.

p4535992 commented 3 years ago

No problem man CUB is much more important. By the way I committed a beta without thinking about it, tonight I'll fix it.

p4535992 commented 3 years ago

K the last commit is a starter integration of the "Point of Interest Teleport" module, for which I wanted to create a series of new features to integrate with pin cushion, so you might as well ignore it if you want. For now I will not update this PR again. Let me know if I need to fix anything for you. Greetings

Shuggaloaf commented 3 years ago

Hi all! I actually came to death-save's git to request that he take up development of backgroundless pins because of it being abandoned. (And the pin cushion dev seemed like the logical place to start)

I saw this PR and couldn't believe it! Excellent job @p4535992 thank you so much for not just having the idea but helping make it happen. And you are right, I do also use journal thumbnails as well. Having all 3 in 1 mod, with reduced conflicts and mods count will be very nice.

Thanks @eclarke12 for grabbing this and incorporating it. I just grabbed the updated CUB and so far it is great! (Thanks for fixing that double concentration issue). I'll be monitoring this for the merge. Looking forward to it.

Shuggaloaf commented 3 years ago

@p4535992 / @eclarke12 May possibly want to consider not including the POI Teleport at this time. I say this hesitantly as I am 100% for combining as many mods as possible to reduce possible conflicts.

There are several other existing methods/mods to accomplish this. Trigger Happy mod can do this very easily just by adding a transparent drawing over the map pin area. When clicked (or optionally when a token is moved to that area), it loads whatever scene you would like it to. It also is not Scene Notes dependent. Also, quite simply with no mods, you can just drag a scene into the journal entry of that map pin and create a link. Click to open map pin journal entry, click scene link, done. Usage is pretty much the same amount of clicks with no mod dependencies and less setup.

Just some thoughts!

Ethaks commented 2 years ago

@p4535992, would you consider the PR stable as is, or do you have plans for further additions etc.? If you consider it to be somewhat done, I'll take another look and possibly change up some stuff/build upon it, before hopefully getting a release ready soon-ish.

p4535992 commented 2 years ago

@Ethaks my PR was born to integrate some deprecated modules that I always used together (even if now they have updated them for 9) and synchronize everything with the wrapper module. Except for issues that are pointed out to me, I don't think I'll get back on it, for me you can try to take a look at it.

p4535992 commented 2 years ago

@eclarke12 @Ethaks just a reminder for this, i made some test and seem working fine

p4535992 commented 2 years ago

Now i'm officially the new maintainer of the module so i'll close this PR

p4535992 commented 2 years ago

I'll close this pr because, i'm now the officialy new maintainer.