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

Revise Develop(ment) tab #149

Merged
merged 11 commits into from
Jan 29, 2019
Merged

Revise Develop(ment) tab #149

merged 11 commits into from
Jan 29, 2019

Conversation

jg18
Copy link
Collaborator

@jg18 jg18 commented Dec 15, 2018

Note: This is a WIP. I wanted to post what I have so far in case you already have feedback.

To explain why I prefer Develop for the tab name over Development, it's shorter and lines up nicely with the verb theme started by Explore. As mentioned inanother PR, I was gonna propose modifying the tab names so that each one is a (relatively) short verb. But that's up for discussion of course.

Btw why is the create mod popup in both kn-page.vue and kn-devel-page.vue ? I just realized I was editing the wrong template if I wanted to update that popup. Are any other dev tab popups duplicated like that, just so I know which page to edit?

Also, are the tabs called "tabs" or "pages" in player-facing text? We ran into this same question in wxL and I forget which one we chose, but picking one term and being consistent in player-facing text is important. I'm inclined towards "tab" myself.

@jg18
Copy link
Collaborator Author

jg18 commented Dec 15, 2018

On second thought, maybe the instructions should go back to gray, to make it cleareer it's palceholder text until you have stuff you're working on. Originally I thought it just made the text harder to read for no apparent reason. Opinion?

Copy link
Owner

@ngld ngld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with renaming the tab to develop. Heh, I've been calling it "dev tab" for quite some time now.
The create mod popup is only in kn-page.vue. The stuff you edited in kn-devel-page.vue is the mod details tab. The mod details tab allows you to edit the mod metadata for existing mods while the create mod popup (obviously) creates new mods. It's not great since it's code duplication and I don't like how cramped the create mod popup is. It shouldn't be too hard to fix that; We could show the mod details tab once the user clicks on "Create Mod" and disable all other mod tabs (FSO, -mod flag, etc.) until the mod has been saved. We just have to be careful to initialise all relevant variables and disable immutable fields (like the mod ID and type). Those fields are currently the easiest way to tell the two screens apart.

The tabs are called tabs. However, at this point we have three different tab bars: The top tab bar (home, explore, develop, ...) and two rows for the dev tab: The hard-coded tabs (FSO, -mod flag, staff, ...) and the one tab for each package.

I agree that the gray text is too hard to read. IIRC an older version displayed the text "No mod selected" to indicate that the text below is a placeholder but someone else suggested changing that since it would display a negative message as the first thing to the user or something like that...

html/templates/kn-devel-page.vue Outdated Show resolved Hide resolved
html/templates/kn-devel-page.vue Outdated Show resolved Hide resolved
html/templates/kn-devel-page.vue Show resolved Hide resolved
@jg18
Copy link
Collaborator Author

jg18 commented Dec 16, 2018

Um, how did I break CI?

@ngld
Copy link
Owner

ngld commented Dec 16, 2018

That was a bug in the CI software. I've updated it and everything's fine now.

@ngld
Copy link
Owner

ngld commented Dec 17, 2018

Looks like this is ready for merging. Anything you want to change before I do that?

@jg18
Copy link
Collaborator Author

jg18 commented Dec 18, 2018

I don't think I revised the help text or commented out the Tool/Extension options in the "create mod" popup yet. I was also hoping to go through more of the mod creation process and see if I have any other thoughts about how to improve the experience Knossos-wise -- was gonna create a (private?) dummy mod for testing called "Awesome Mod" following the adventures of the GTC Awesome or something.

I did notice that if Knossos can't connect to Nebula to verify that your Mod ID is unique, it warns you with a "use at your own risk" QMessageBox popup but there's only an OK button and no way to back out! 😛

@jg18
Copy link
Collaborator Author

jg18 commented Dec 18, 2018

Btw, not sure who owns the Mod Creation Gudie but there really should be access controls on editing, probably read-only publicly but write access given only to specific Google accounts. We don't want to open the door to vandalism.

@jg18
Copy link
Collaborator Author

jg18 commented Dec 18, 2018

Also, is it worth including in the dev-instructions div links to resources in addition tothe mdo creation guide, like FS Wiki portals on modding or FREDding or things like that? Or maybe there are forum threads worth listing as a resource?

knossos/runner.py Show resolved Hide resolved
@jg18
Copy link
Collaborator Author

jg18 commented Dec 22, 2018

I was wondering why executables are stored in selected_pkg and not selected_mod. Aren't exes per-mod and not per-mod package?

Also the user-facing text uses "executable" and "build" interchangeably, which isn't ideal but I guess the user needs to know/learn that they're synonymous at least when it comes to FSO.

Copy link
Owner

@ngld ngld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re: Guide links, not sure. Should probably asks actual modders either in the forum thread or on Discord. The guide was initially created by Axem who still owns the document. Will have to ask him if he can make a read-only link or something like that.
Otherwise, I could make a static copy on fsnebula.org and link to that. That means we'd have to manually update it but that also means there's no way to vandalize it.

knossos/runner.py Show resolved Hide resolved
A <em>package</em> is a folder for mod data (missions, new mdoels, etc.).
Most mods need only one package that contains all of the mod's data.
Splitting the data into multiple packages can be useful for larger mods,
which might have a main package and optional packages for voice acting or high-detail models.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Important notes here:

  • Hard limitation of 2 GiB per package due to limitations of VP files
  • Each package is packed into one archive. Knossos doesn't resume failed downloads (it restarts them) so splitting huge packages into smaller ones can help users with a unreliable internet connection.
  • During uploading only modified packages are uploaded. If you have a separate package for your missions, updates that only change missions will result in small updates since the models, textures, etc. don't have to be redownloaded. This feature is currently broken though due to a mistake on my part.

These are mostly issues on Knossos side that the user shouldn't have to deal with (even the VP limitation can be worked around by having Knossos build multiple VPs per package in that case). I'm just noting this here since this is already known to a few modders.
Ideally, we should fix these issues instead of teaching people workarounds. That'll take time though. Not sure what the best course of action here is.

</p>
<p>
<!-- TODO consistent terminology! what's the official term for Nebula? mod repository? server? Also what is its user-facing name? Nebula? FSNebula? FS Nebula? -->
<!-- Just because the URL says fsnebula doesn't mean we have to call it "FS Nebula". I'm a fan of just calling it "Nebula". -->
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just call it Nebula (unsurprisingly, nebula.org was taken so I had to settle for fsnebula.org). Knossos should consistently call it Nebula as well. I'm not entirely shure though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, in my revisions should I refer to Nebula as just "Nebula"? Or "Nebula mod repository" (or "Nebula server"?) on first reference and then just "Nebula" after that?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just Nebula should be fine. We probably need an FAQ for stuff like packages anyway. We can explain what Nebula means there.

QtWidgets.QMessageBox.critical(
None, 'Knossos',
translate('runner', 'No executable was found for this mod!'
' Make sure you have an FSO build selected on the FSO tab.'))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A huge problem with this is that this can show up for end users as well under certain conditions (I know the original error message isn't much better). However, they won't know which FSO tab you're talking about.

i want to eventually rework the way engine dependencies work by splitting them out of the normal dependencies and storing them directly on the mod object. This will make them a special case but that allows us to handle a bunch of problematic cases much better (like this one).
Until then, checking mod.dev_mode is a good way to check whether the user is a dev or not.

Possible reasons this could happen on a normal system / user installation:

  • The user is on a 32bit Linux. We only have 64bit Linux builds thus it's impossible to find a matching build. IIRC they won't be able to install mods so they might not come this far. However, if they somehow managed to install a mod (possibly by copying the library folder from a different computer), they'll see this error.
  • Dependency issues which will cause Knossos to be unable to find the engine mod (horrible naming, I know).
  • Some kind of installation issues with the engine (corrupt / missing files, etc.).

@jg18
Copy link
Collaborator Author

jg18 commented Dec 23, 2018

Thanks for the feedback. I'll work on it during the day (it's after midnight here).

Some admittedly out-of-scope thoughts:

While I was waiting for feedback I tried implementing the sorting menu but the result was a horribly hackish half-working mess. I kept a gist of the changes in case any of it is useful: https://gist.github.com/jg18/735da1690bfd03f292bbf163cdb7ce18

Also, I noticed after installing a few mods that whenever I switch to the Explore tab, there's a 2 second delay before it comes up. Why is that? It's kind of annoying IMHO. I'm guessing it's from having to run update_mod_list() every time we switch to Explore? is there a way we can cache the Explore mod data either Vue-side or Python-side and just swap it in while updating HellWindow in the background (e.g., calling update_mod_list() without emitting a Qt signal)?

I also noticed this 2-second delay when trying to implement the sorting menu. To make the sorting not so sluggish, IMO we need to optimize this, e.g., by doing the sort JS-side while performing the same sort Python-side (update_mod_list()) without emitting a Qt signal.

@jg18
Copy link
Collaborator Author

jg18 commented Dec 23, 2018

As for modding resource links, I'll ask on #knossoson Discord and ask them to PM us on HLP.

As for the Mod Creation Guide, I'm pretty sure Google Docs supports providing a read-only link to a doc while restricting write access to a specified list of Google accounts. IIRC my netcode doc is set up that way. I'll PM Axem and ask him to set up the guide that way.

Will get to your other comments in a bit...

EDIT: Actually thinking #freespace would make more sense than #knossos to ask so will do that. #knossos probably has a lot of Knossos users who aren't modders and I think the SNR would be higher in #freespace, could eb wrong though.

@jg18
Copy link
Collaborator Author

jg18 commented Dec 24, 2018

The latest commits should address everything except getting a read-only link to the mod creation guide (I PMed Axem on HLP about it, waiting for a response) and maybe adding some more resource links. The PR doesn't cover all of the changes I might want to propose but it's a great start. 🙂

Listening to the Dev Help section with the screen reader uncovered a number of spelling errors 🙁 but at least they're fixed now.

Copy link
Owner

@ngld ngld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Once the new guide link is in, I'd say we can merge this.

</p>
<ul>
<li>A VP file can be at most 2 GB.</li>
<li>Each compressed archive file on Nebula contains a single VP file.<br>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only true if the uploader uses Knossos' automatic VP packing. If they manually build VP files and put them in packages, each package (and thus archive) can contain more than one VP file. Not really a problem, rather a nitpicky detail.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm a little confused. By "automatic VP packing" you mean the entire process from package folder to VP file to .7z archive file, right? Yeah I suppose that if you create the VP files and then pack the VP files into compressed archive files yourself, you can pack the data however you like. I'll see if I can come up with a succinct way of explaining that. I'm not sure we should encourage people to VPify/compress the data themselves when they might mess it up and Knossos can do it painlessly for them.

@jg18
Copy link
Collaborator Author

jg18 commented Jan 2, 2019

Ok, mod creation guide link is now read-only access. Assuming the text revision in the new commit looks good to you. this PR is good to go. 🙂 Then we can wrap up the retail install prompt PR and then the delta updates and finally the sorting menu.

@jg18
Copy link
Collaborator Author

jg18 commented Jan 4, 2019

Given what Orpheus said on the release thread about need to clarify I can make that text revision before you merge this PR. Can probably commit the change this weekend.

@jg18
Copy link
Collaborator Author

jg18 commented Jan 11, 2019

Ok I came up with some new user-facing messages as per 0rph3u5's recommendation and he said the new messages look good to him. I made a few small revisions while I was at it. Assuming you're happy with all of the changes, I'd say this is ready to merge.

@ngld ngld merged commit f227afe into ngld:develop Jan 29, 2019
@ngld
Copy link
Owner

ngld commented Jan 29, 2019

Looks good to me! Thanks for your hard work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants