Pull even more of the subs load logic into Project

The relative order of the on-file-load logic matters, so signals don't
work very well for it. Doing it all explicitly in Project fixes a bunch
of inconsistent issues related to restoring saved state.

Closes #1759.

src/base_grid.cpp

@@ -120,7 +120,6 @@ BaseGrid::BaseGrid(wxWindow* parent, agi::Context *context)
 
 	connections = agi::signal::make_vector({
 		context->ass->AddCommitListener(&BaseGrid::OnSubtitlesCommit, this),
-		context->subsController->AddFileOpenListener(&BaseGrid::OnSubtitlesOpen, this),
 
 		context->selectionController->AddActiveLineListener(&BaseGrid::OnActiveLineChanged, this),
 		context->selectionController->AddSelectionListener([&]{ Refresh(false); }),
@@ -176,10 +175,6 @@ void BaseGrid::OnSubtitlesCommit(int type) {
 	}
 }
 
-void BaseGrid::OnSubtitlesOpen() {
-	ScrollTo(context->ass->Properties.scroll_position);
-}
-
 void BaseGrid::OnShowColMenu(wxCommandEvent &event) {
 	int item = event.GetId() - MENU_SHOW_COL;
 	bool new_value = !columns_visible[item];

src/base_grid.h

@@ -92,11 +92,8 @@ class BaseGrid final : public wxWindow {
 	void OnShowColMenu(wxCommandEvent &event);
 	void OnSize(wxSizeEvent &event);
 	void OnSubtitlesCommit(int type);
-	void OnSubtitlesOpen();
 	void OnActiveLineChanged(AssDialogue *);
 
-	void ScrollTo(int y);
-
 	void AdjustScrollbar();
 	void SetColumnWidths();
 
@@ -120,6 +117,7 @@ public:
 	~BaseGrid();
 
 	void SetByFrame(bool state);
+	void ScrollTo(int y);
 
 	DECLARE_EVENT_TABLE()
 };

src/frame_main.cpp

@@ -149,7 +149,7 @@ FrameMain::FrameMain()
 	SetDropTarget(new AegisubFileDropTarget(context.get()));
 
 	StartupLog("Load default file");
-	context->subsController->Close();
+	context->project->CloseSubtitles();
 
 	StartupLog("Display main window");
 	AddFullScreenButton(this);

src/project.cpp

@@ -16,9 +16,11 @@
 
 #include "project.h"
 
+#include "ass_dialogue.h"
 #include "ass_file.h"
 #include "async_video_provider.h"
 #include "audio_controller.h"
+#include "base_grid.h"
 #include "charset_detect.h"
 #include "compat.h"
 #include "dialog_progress.h"
@@ -29,7 +31,9 @@
 #include "include/aegisub/video_provider.h"
 #include "mkv_wrap.h"
 #include "options.h"
+#include "selection_controller.h"
 #include "subs_controller.h"
+#include "utils.h"
 #include "video_controller.h"
 #include "video_display.h"
 
@@ -95,17 +99,18 @@ void Project::SetPath(agi::fs::path& var, const char *token, const char *mru, ag
 	UpdateRelativePaths();
 }
 
-void Project::DoLoadSubtitles(agi::fs::path const& path, std::string encoding) {
+bool Project::DoLoadSubtitles(agi::fs::path const& path, std::string encoding, ProjectProperties &properties) {
 	try {
 		if (encoding.empty())
 			encoding = CharSetDetect::GetEncoding(path);
 	}
 	catch (agi::UserCancelException const&) {
-		return;
+		return false;
 	}
 	catch (agi::fs::FileNotFound const&) {
 		config::mru->Remove("Subtitle", path);
-		return ShowError(path.string() + " not found.");
+		ShowError(path.string() + " not found.");
+		return false;
 	}
 
 	if (encoding != "binary") {
@@ -113,48 +118,67 @@ void Project::DoLoadSubtitles(agi::fs::path const& path, std::string encoding) {
 		// distinguish them based on filename alone, and just ignore failures
 		// rather than trying to differentiate between malformed timecodes
 		// files and things that aren't timecodes files at all
-		try { return DoLoadTimecodes(path); } catch (...) { }
-		try { return DoLoadKeyframes(path); } catch (...) { }
+		try { DoLoadTimecodes(path); return false; } catch (...) { }
+		try { DoLoadKeyframes(path); return false; } catch (...) { }
 	}
 
 	try {
-		context->subsController->Load(path, encoding);
+		properties = context->subsController->Load(path, encoding);
 	}
-	catch (agi::UserCancelException const&) { return; }
+	catch (agi::UserCancelException const&) { return false; }
 	catch (agi::fs::FileNotFound const&) {
 		config::mru->Remove("Subtitle", path);
-		return ShowError(path.string() + " not found.");
+		ShowError(path.string() + " not found.");
+		return false;
 	}
 	catch (agi::Exception const& e) {
-		return ShowError(e.GetMessage());
+		ShowError(e.GetMessage());
+		return false;
 	}
 	catch (std::exception const& e) {
-		return ShowError(std::string(e.what()));
+		ShowError(std::string(e.what()));
+		return false;
 	}
 	catch (...) {
-		return ShowError(wxString("Unknown error"));
+		ShowError(wxString("Unknown error"));
+		return false;
 	}
+
+	Selection sel;
+	AssDialogue *active_line = nullptr;
+	if (!context->ass->Events.empty()) {
+		int row = mid<int>(0, properties.active_row, context->ass->Events.size() - 1);
+		active_line = &*std::next(context->ass->Events.begin(), row);
+		sel.insert(active_line);
+	}
+	context->selectionController->SetSelectionAndActive(std::move(sel), active_line);
+	context->subsGrid->ScrollTo(properties.scroll_position);
+
+	return true;
 }
 
 void Project::LoadSubtitles(agi::fs::path const& path, std::string encoding) {
-	DoLoadSubtitles(path, encoding);
-	LoadUnloadFiles();
+	ProjectProperties properties;
+	if (DoLoadSubtitles(path, encoding, properties))
+		LoadUnloadFiles(properties);
 }
 
 void Project::CloseSubtitles() {
 	context->subsController->Close();
 	config::path->SetToken("?script", "");
-	LoadUnloadFiles();
+	LoadUnloadFiles(context->ass->Properties);
+	auto line = &*context->ass->Events.begin();
+	context->selectionController->SetSelectionAndActive({line}, line);
 }
 
-void Project::LoadUnloadFiles() {
+void Project::LoadUnloadFiles(ProjectProperties properties) {
 	auto load_linked = OPT_GET("App/Auto/Load Linked Files")->GetInt();
 	if (!load_linked) return;
 
-	auto audio     = config::path->MakeAbsolute(context->ass->Properties.audio_file, "?script");
-	auto video     = config::path->MakeAbsolute(context->ass->Properties.video_file, "?script");
-	auto timecodes = config::path->MakeAbsolute(context->ass->Properties.timecodes_file, "?script");
-	auto keyframes = config::path->MakeAbsolute(context->ass->Properties.keyframes_file, "?script");
+	auto audio     = config::path->MakeAbsolute(properties.audio_file, "?script");
+	auto video     = config::path->MakeAbsolute(properties.video_file, "?script");
+	auto timecodes = config::path->MakeAbsolute(properties.timecodes_file, "?script");
+	auto keyframes = config::path->MakeAbsolute(properties.keyframes_file, "?script");
 
 	if (video == video_file && audio == audio_file && keyframes == keyframes_file && timecodes == timecodes_file)
 		return;
@@ -189,14 +213,14 @@ void Project::LoadUnloadFiles() {
 			CloseVideo();
 		else if ((loaded_video = DoLoadVideo(video))) {
 			auto vc = context->videoController.get();
-			vc->JumpToFrame(context->ass->Properties.video_position);
+			vc->JumpToFrame(properties.video_position);
 
-			auto ar_mode = static_cast<AspectRatio>(context->ass->Properties.ar_mode);
+			auto ar_mode = static_cast<AspectRatio>(properties.ar_mode);
 			if (ar_mode == AspectRatio::Custom)
-				vc->SetAspectRatio(context->ass->Properties.ar_value);
+				vc->SetAspectRatio(properties.ar_value);
 			else
 				vc->SetAspectRatio(ar_mode);
-			context->videoDisplay->SetZoom(context->ass->Properties.video_zoom);
+			context->videoDisplay->SetZoom(properties.video_zoom);
 		}
 	}
 
@@ -475,13 +499,11 @@ void Project::LoadList(std::vector<agi::fs::path> const& files) {
 			audio = file;
 	}
 
-	if (!subs.empty())
-		DoLoadSubtitles(subs);
-
-	// Loading video will clear the audio file script header, so make sure we
-	// end up loading the audio if the newly loaded subs has some
-	if (!video.empty() && audio.empty() && !context->ass->Properties.audio_file.empty())
-		audio = config::path->MakeAbsolute(context->ass->Properties.audio_file, "?script");
+	ProjectProperties properties;
+	if (!subs.empty()) {
+		if (!DoLoadSubtitles(subs, "", properties))
+			subs.clear();
+	}
 
 	if (!video.empty()) {
 		DoLoadVideo(video);
@@ -508,5 +530,5 @@ void Project::LoadList(std::vector<agi::fs::path> const& files) {
 		DoLoadAudio(video_file, true);
 
 	if (!subs.empty())
-		LoadUnloadFiles();
+		LoadUnloadFiles(properties);
 }

src/project.h

@@ -27,6 +27,7 @@ class AudioProvider;
 class DialogProgress;
 class wxString;
 namespace agi { struct Context; }
+struct ProjectProperties;
 
 class Project {
 	std::unique_ptr<::AudioProvider> audio_provider;
@@ -51,13 +52,13 @@ class Project {
 	void ShowError(wxString const& message);
 	void ShowError(std::string const& message);
 
-	void DoLoadSubtitles(agi::fs::path const& path, std::string encoding="");
+	bool DoLoadSubtitles(agi::fs::path const& path, std::string encoding, ProjectProperties &properties);
 	void DoLoadAudio(agi::fs::path const& path, bool quiet);
 	bool DoLoadVideo(agi::fs::path const& path);
 	void DoLoadTimecodes(agi::fs::path const& path);
 	void DoLoadKeyframes(agi::fs::path const& path);
 
-	void LoadUnloadFiles();
+	void LoadUnloadFiles(ProjectProperties properties);
 	void UpdateRelativePaths();
 	void ReloadAudio();
 	void ReloadVideo();

src/selection_controller.cpp

@@ -20,27 +20,10 @@
 #include "ass_file.h"
 #include "include/aegisub/context.h"
 #include "subs_controller.h"
-#include "utils.h"
 
 #include <algorithm>
 
-SelectionController::SelectionController(agi::Context *c)
-: context(c)
-, open_connection(c->subsController->AddFileOpenListener(&SelectionController::OnSubtitlesOpen, this))
-{
-}
-
-void SelectionController::OnSubtitlesOpen() {
-	selection.clear();
-	active_line = nullptr;
-	if (!context->ass->Events.empty()) {
-		int row = mid<int>(0, context->ass->Properties.active_row, context->ass->Events.size() - 1);
-		active_line = &*std::next(context->ass->Events.begin(), row);
-		selection.insert(active_line);
-	}
-	AnnounceSelectedSetChanged();
-	AnnounceActiveLineChanged(active_line);
-}
+SelectionController::SelectionController(agi::Context *c) : context(c) { }
 
 void SelectionController::SetSelectedSet(Selection new_selection) {
 	selection = std::move(new_selection);

src/selection_controller.h

@@ -46,11 +46,6 @@ class SelectionController {
 	Selection selection; ///< Currently selected lines
 	AssDialogue *active_line = nullptr; ///< The currently active line or 0 if none
 
-	agi::signal::Connection open_connection;
-
-	void OnSubtitlesOpen();
-	void OnSubtitlesSave();
-
 public:
 	SelectionController(agi::Context *context);
 

src/subs_controller.cpp

@@ -173,7 +173,7 @@ void SubsController::SetSelectionController(SelectionController *selection_contr
 	selection_connection = context->selectionController->AddSelectionListener(&SubsController::OnSelectionChanged, this);
 }
 
-void SubsController::Load(agi::fs::path const& filename, std::string charset) {
+ProjectProperties SubsController::Load(agi::fs::path const& filename, std::string charset) {
 	AssFile temp;
 
 	SubtitleFormat::GetReader(filename, charset)->ReadFile(&temp, filename, context->project->Timecodes(), charset);
@@ -185,6 +185,7 @@ void SubsController::Load(agi::fs::path const& filename, std::string charset) {
 		temp.Events.push_back(*new AssDialogue);
 
 	context->ass->swap(temp);
+	auto props = context->ass->Properties;
 
 	SetFileName(filename);
 
@@ -207,6 +208,7 @@ void SubsController::Load(agi::fs::path const& filename, std::string charset) {
 	}
 
 	FileOpen(filename);
+	return props;
 }
 
 void SubsController::Save(agi::fs::path const& filename, std::string const& encoding) {

src/subs_controller.h

@@ -21,10 +21,10 @@
 #include <boost/filesystem/path.hpp>
 #include <wx/timer.h>
 
-struct AssFileCommit;
 class SelectionController;
-
 namespace agi { struct Context; }
+struct AssFileCommit;
+struct ProjectProperties;
 
 class SubsController {
 	agi::Context *context;
@@ -82,7 +82,7 @@ public:
 	/// @brief Load from a file
 	/// @param file File name
 	/// @param charset Character set of file
-	void Load(agi::fs::path const& file, std::string charset);
+	ProjectProperties Load(agi::fs::path const& file, std::string charset);
 
 	/// @brief Save to a file
 	/// @param file Path to save to