AdamsLair / duality

a 2D Game Development Framework
https://adamslair.github.io/duality
MIT License
1.4k stars 290 forks source link

Installed sample project's "Data" / content files show up in VS #845

Open ilexp opened 4 years ago

ilexp commented 4 years ago

Summary

Just tested the new v4-alpha2 Duality project template and installed the Tilemaps sample package into the GameEditor project. Everything worked as expected, but there is one issue that might easily throw off new users and could also annoy experienced users:

devenv_UCBu1StmlS

All the installed content files actually show up in the project in Visual Studio, and they can be edited via double-click. That alone would be tolerable, but any changes made are simply ignored and don't end up in the actual project.

There are a lot of internal theories on how Duality works that new users might develop based on that, most of which would lead them into the wrong direction, so I'm flagging this as a usability bug. If possible, we should fix that in some way before the actual release.

Analysis

Here's a number of ways this could be fixed, though I don't really know how difficult any of them would be - @Barsonax probably has a better idea on that topic.

The last one is probably a bad idea, since that would mean letting users edit the content files in their local package cache directory. I'd probably go with the first option to avoid any potential confusion in the first place, if there is a way to achieve that.

Barsonax commented 4 years ago

Yes I noticed this before. The fact that they are editable seems to be some issue with the tooling as it should be readonly. Nothing we can do about that iam afraid.

As I don't think this is very pretty I think we should find a way to hide these files. They should still be included in the project though else they won't get copied to the output.

Barsonax commented 4 years ago

Well seems you are not allowed to change any properties of contentfiles so I don't currently see a way to solve this.

Barsonax commented 4 years ago

I think this is more a issue with visual studio handling of contentfiles in the solution explorer. Microsoft has chosen to dump everything in the root which is not very clear. It would be much clearer if it was listed under Dependencies under the respective package.

So I think we should make a issue about this (not sure which repo houses visual studio related issues) but until then we have to live with this.

EDIT: its still a open issue https://github.com/NuGet/Home/issues/4856

ilexp commented 4 years ago

Good find 👍 Skimming through the linked thread, could any of the workarounds help in the meantime? This one seems to hide files otherwise included, while this one is supposed to group them into a folder.

Barsonax commented 4 years ago

Well its not pretty and doesn't completely solve this:

    <None Update="@(None)">
      <Link Condition="'%(NuGetPackageId)' != ''">%(NuGetPackageId)\%(Link)</Link>
      <Visible>false</Visible>
    </None>
    <Resource Update="@(Resource)">
      <Link Condition="'%(NuGetPackageId)' != ''">%(NuGetPackageId)\%(Link)</Link>
      <Visible>false</Visible>
    </Resource>
    <Content Update="@(Content)">
      <Link Condition="'%(NuGetPackageId)' != ''">%(NuGetPackageId)\%(Link)</Link>
      <Visible>false</Visible>
    </Content>

Result (folders still visible): image

EDIT: that Link actually also changes the folder where its deployed so that might not be preferable EDIT: another issue about being able to edit the files https://github.com/dotnet/project-system/issues/2141 EDIT: created another issue about this https://github.com/dotnet/project-system/issues/6290

ilexp commented 4 years ago

Result (folders still visible):

Ouch 😄 Thanks for checking though. Seems like this is tougher than I expected, let's hope we get a tooling update soon. Until then, we should mention that stuff in the docs when getting to the articles on the topic.

EDIT: another issue about being able to edit the files dotnet/project-system#2141 EDIT: created another issue about this dotnet/project-system#6290

👍

Barsonax commented 4 years ago

Well seems the fact that files show up in the tree like that is 'by design' and thus won't be fixed...

Atleast they recognized that being able to edit the files is a bug.

ilexp commented 4 years ago

Hm, I guess it makes sense when assuming that all package contents are deliberate user-side additions that are managed through VS - in that regard our use case just isn't the mainstream one. Still, it would be nice if there was some way to control this.

Package authors should be using <Visible>false</Visible> on these content files if they are not intended to be shown in the project, I'll continue to let NUGet track adding a feature to control this via NuGet/Home#4856.

Assuming this is different from the Visible hack in the csproj, can this help us? Or would that break the copy step?

I'm not 100% sure where this visible tag would be added on the package side, but since we are in control of the packages we release, and third-party authors could adhere to our guidelines for packages with content files, that might be a solution if it works.

Edit: I'm not finding any way to specify visibility in the .nuspec / on the package author side - probably missing something here?

Barsonax commented 4 years ago

Hm, I guess it makes sense when assuming that all package contents are deliberate user-side additions that are managed through VS - in that regard our use case just isn't the mainstream one. Still, it would be nice if there was some way to control this.

Haha that doesn't explain all the issues that are made about this though :P

Assuming this is different from the Visible hack in the csproj, can this help us? Or would that break the copy step?

I don't understand what he is trying to say. <Visible> is for the consuming project and not for the project that creates the package. I think he is mixing things up.

We could probably come up with some csproj hack on the consuming project side that hides everything in the Data folder if this gets fixed https://developercommunity.visualstudio.com/idea/1079955/parent-folder-of-items-marked-as-visiblefalse-stil.html. Its not gonna be pretty though and controlling this from the nuget package would be better I think as the nuget package knows the intention of the files better.

ilexp commented 4 years ago

Alternatively, if the readonly fix is out there reasonably fast, that would be okay as well. As long as users can't edit these files, they'll get the idea that this is just something "for reference" and hopefully ignore them. Putting the file links into a subfolder named after the package like you did in the earlier experiment would also improve this.