Add update load combination to adapter push

[Chrisshort92]

NOTE: Depends on

Issues addressed by this PR

Closes #563

Implements Update functionality for LoadCombinations in Robot_Adapter. The Update method now:

• Locates existing combinations using the Number property via direct Get() call (following Loadcases pattern)
• Updates combination names and replaces case factors
• Validates RobotId/Number consistency and logs errors on mismatch
• Provides clear warnings when combinations don't exist in Robot

Test files

https://burohappold.sharepoint.com/:f:/r/sites/BHoM/02_Current/12_Scripts/02_Pull%20Request/BHoM/Robot_Toolkit/%23565-add-update-load-combination-to-adapter-push?csf=1&web=1&e=GLPz1q

Changelog

Update functionality for LoadCombinations in Robot_Adapter

Additional comments

[staintono]

Tested using the provided test script, load combinations are updated as expected based on the combination number. Error is thrown if combination does not yet exist in the model, as expected.

[peterjamesnugent]

Can you update the title, folder for the test script and the text for the PR. The body mentions Create methods breaking but does not address it in the PR. Similar for the Fixed Update Method and Technical Improvements, not sure how much it relates to the PR changes. I would keep the change log and description to a minimum for clarity.

I ran the script, but it just tried to update the RobotId for LoadCombination to 100, but it's already set as 100 so it doesn't really test the Update. I tried changing it to 200 and nothing happens:

In your script I would also want to a case that updates the factors, a case that updates the Loadcase and a case that tries to update with empty cases (prompting the error).

[staintono]

@copilot update the PR description to summarise the current state of the changes.

[Copilot]

@staintono I've opened a new pull request, #575, to work on those changes. Once the pull request is ready, I'll request review from you.

[staintono]

Can you update the title, folder for the test script and the text for the PR. The body mentions Create methods breaking but does not address it in the PR. Similar for the Fixed Update Method and Technical Improvements, not sure how much it relates to the PR changes. I would keep the change log and description to a minimum for clarity.

I ran the script, but it just tried to update the RobotId for LoadCombination to 100, but it's already set as 100 so it doesn't really test the Update. I tried changing it to 200 and nothing happens:

In your script I would also want to a case that updates the factors, a case that updates the `Loadcase` and a case that tries to update with empty cases (prompting the error).

PR Title and Folder have been updated. Updated the code to include errors where the RobotId doesnt equal the Loadcombination Number, it wont update unless they are both the same. Test script has been updated to include updating just factors or load cases.

[staintono]

@BHoMBot Check Compliance

[Chrisshort92]

I have just re-tested this functionality and works as expected with the update functions @staintono states above. @peterjamesnugent can you re-review ready for merge?

[peterjamesnugent]

Some changes that were not covered in the test scripts, handling of null and where to exit the routine as well as warning and error messaging.

[peterjamesnugent]

Suggested change

	int robotId = GetAdapterId<int>(lComb);
	int robotId = Query.HasAdapterIdFragment(lComb, typeof(RobotId)) ? GetAdapterId<int>(lComb) : 0;

You should check if lComb has a Fragment first, otherwise you will get two warnings/notes for the same thing:

[peterjamesnugent]

Suggested change

	Engine.Base.Compute.RecordNote($"LoadCombination with number {combinationId} did not have a RobotId. RobotId has been set to the LoadCombination number.");
	Engine.Base.Compute.RecordWarning($"LoadCombination with number {combinationId} did not have a RobotId. RobotId has been set to the LoadCombination number.");

Warning given you have made changes to the users object.

[peterjamesnugent]

Suggested change

	Engine.Base.Compute.RecordWarning("Load combination with number " + combinationId.ToString() + " does not exist in Robot. Load combination could not be updated!");
	success = false;
	continue;
	Engine.Base.Compute.RecordError("Load combination with number " + combinationId.ToString() + " does not exist in Robot. Load combination could not be updated!");
	return false;

Suggest we return an error here and exit the method given you cannot do anything with a null rCaseCombination.

[peterjamesnugent]

Why not put this inside the if statement below? That way you don't clear all the factors of a LoadCombination that has no Loadcases.

[peterjamesnugent]

Suggested change

	Engine.Base.Compute.RecordWarning("Load combination with number " + combinationId.ToString() + " has no load cases. The combination has been cleared of all case factors.");
	Engine.Base.Compute.RecordWarning("Load combination with number " + combinationId.ToString() + " has no load cases.");

Then we don't delete the factors.

[peterjamesnugent]

If it's null don't we want to return a error stating as such - and then returning false?

[peterjamesnugent]

Suggested change

	Engine.Base.Compute.RecordError($"Load combination has mismatched IDs: RobotId = {robotId}, Number = {combinationId}. Using Number property for update.");
	Engine.Base.Compute.RecordWarning($"Load combination has mismatched IDs: RobotId = {robotId}, Number = {combinationId}. Using Number property for update.");

I would make this a warning, to make the user aware of a change of approach. Errors are usually when something is not going to work.

[peterjamesnugent]

If you add a using BH.oM.Base.Debugging; statement you can simplify this.

I would also add using
using BH.oM.Adapters.Robot; using BH.Engine.Adapter;

[peterjamesnugent]

Is this line needed? You have already updated the object id when it didn't match the load combination number?