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

Allow players to join as dm - with some minor differences either due to DnDBeyond permissions or stuff we save local still (eg. notes require export/import since still local) #758

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Azmoria
Copy link
Collaborator

@Azmoria Azmoria commented Jan 9, 2023

This mostly seems to work as expected again after the changes that have been made since removal of this feature. I think it would be worth while to enable this feature again - it does get requested semi-often.

Things that work:

  • Scenes
  • Monster sheets
  • Tokens already placed on map (not notes requires export/import)
  • Combat tracker
  • Drawing, fog and other tools

Some differences other than the above:

  • Players still only have access to view other player sheets as they would on DnDBeyond
  • Journals, audio are still saved locally so exports/imports might be required to move these between DM's.
  • Token customizations (similar to above stored locally - requires import/export)

@vlaminck
Copy link
Collaborator

vlaminck commented Feb 6, 2023

This does mostly work, but it comes with enough caveats, that we should put a little more work into it. The last thing that I want is for people to ask in the Discord why things don't show up. We should put things in the app to explain to the user how to resolve these scenarios so they don't have to ask in the Discord.

  1. Ideally, I think the campaign owner should have to specify 1 character to be the DM, and only then would that player gain access to the DM tools. The owner of the campaign should also be able to remove that as well. That would require some work on the campaign page, and I actually don't even know if we could store that as a settings somewhere in the cloud to make it stick across sessions. If we don't build out this sort of system we should at least hide the "join as dm" button somewhere in the "Instructions" like it used to be. We don't want to advertise to everyone that they can log in as the DM and look at all the maps.

  2. We used to alert the currently connected DM that another DM joined, and then we kicked them out. I don't know if we want to do that or not. I like the idea of allowing a co-DM situation, but I also don't know if having multiple DMs connected at the same time would cause any weird side effects with cloud communication, data saving, etc. It seems to work ok, so I'm inclined to leave it as is, I just want us to at least think about this as an option.

  3. When a player joins as DM, we should also explain to them all the things that are not stored in the cloud, and that they would need the campaign owner to export their data so that this player can import all those things. If we had everything in the cloud, this wouldn't be an issue, but we're just not there yet unfortunately.

  4. I think the player sheet permissions are enough to cause a lot of confusion. For example, in the screenshot below, the left side of this image is the owner of the campaign, and the right side is a separate DDB account that is a player. The account on the right does not have permission to view the other account's character so that character just doesn't show up in the list. In this example, the character "Bladesinger" is missing. To resolve this, the account that owns the "Bladesinger" character would need to go to the "HOME" tab within the character builder and change the "Character Privacy" to "Public". A quick glance at the network requests doesn't show any errors for that character either. I'm sure we could figure out which characters are missing, but I'm not sure off the top of my head how.
    For DMs that don't own the campaign, we should show a message in the players section of the sidebar tab with a message that says "Missing characters?" that then tells the user how to resolve this problem. If we could figure out which characters are missing from the list, we could put a placeholder in the list to make it super obvious that characters are missing.

avtt-player-as-dm-permissions-issue

@vlaminck vlaminck added the needwork Work, work, work, work, work, work He said me haffi work, work, work, work, work, work He see me do label Feb 6, 2023
@Azmoria
Copy link
Collaborator Author

Azmoria commented Feb 6, 2023

Yeah that makes sense. I'm going to put this on the back burner for a bit. Especially with cloud work temporarily on hold.

1 is tough we might be able to move it into the campaign instead (assuming we have access to the same data or the DM can provide it when they allow the character to join as dm). I agree that at least moving it inside the instructions for the players makes sense with a warning near by that specifies any inconsistencies.

I did test 2 a bit and didn't see any immediate issues but there definitely/likely could be some - like if they both went to play sounds I don't know if they would override each other or play on top of each other. I didn't really do any creation of stuff there either. I like the idea of being able to co-dm as well.

3 agreed

4 yeah it's one of the things on my list that doesn't work the same. They'd have to make sheets public. On the join message we could check if player_name is in the player folder - if not add a message there?

@Azmoria Azmoria marked this pull request as draft February 6, 2023 14:56
@vlaminck
Copy link
Collaborator

I haven't forgotten about this. I have some more cleanup that I want to do in the startup process, and then I'll take a pass at this.

@github-actions
Copy link

github-actions bot commented Mar 1, 2023

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@vlaminck vlaminck added the enhancement New feature or request label Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Conflicting enhancement New feature or request needwork Work, work, work, work, work, work He said me haffi work, work, work, work, work, work He see me do
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants